Kelley Nielsen
2013-Nov-11 15:50 UTC
[PATCH OPW 0/0] Resend of macro coding style fixes needing reindents
These patches all use the convenience macros defined in libxl_internals.h to replace the old style. I''ve been asked to correct indentation errors that were present in the first versions and resend them all as a set. [PATCH OPW 1/7] libxl: Use new macro LOGE() in libxl_qmp.c [PATCH OPW 2/7] libxl: replace last invocation of LIBXL__LOG_ERROR [PATCH OPW 3/7] libxl: use GCSPRINTF in place of libxl_sprintf() in [PATCH OPW 4/7] libxl: use GCSPRINTF instead of libxl__sprintf [PATCH OPW 5/7] libxl: use LOGE instead of LIBXL__LOG_ERRNO in [PATCH OPW 6/7] libxl: use LOG and LOGE instead of LIBXL__LOG* in [PATCH OPW 7/7] libxl: use LOG instead of LIBXL__LOG in libxl_utils.c
Kelley Nielsen
2013-Nov-11 15:50 UTC
[PATCH OPW 1/7] libxl: Use new macro LOGE() in libxl_qmp.c
Code cleanup -- no functional changes Coding style has recently been changed for libxl. The convenience macro LOGE() has been introduced, and invocations of the old macro LIBXL__LOG_ERROR() are to be replaced with it. Change all occurences of the old macro (in functions that have a local libxl_gc *gc) except the one in register_serials_chardev_callback() to the new one. (This function lacks a local libxl__gc *gc, which LOGE() requires.) Suggested-by: Anthony PERARD <anthony.perard@citrix.com> Signed-off-by: Kelley Nielsen <kelleynnn@gmail.com> --- tools/libxl/libxl_qmp.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c index 347ff9b..a181383 100644 --- a/tools/libxl/libxl_qmp.c +++ b/tools/libxl/libxl_qmp.c @@ -354,8 +354,7 @@ static libxl__qmp_handler *qmp_init_handler(libxl__gc *gc, uint32_t domid) qmp = calloc(1, sizeof (libxl__qmp_handler)); if (qmp == NULL) { - LIBXL__LOG_ERRNO(libxl__gc_owner(gc), LIBXL__LOG_ERROR, - "Failed to allocate qmp_handler"); + LOGE(ERROR, "Failed to allocate qmp_handler"); return NULL; } qmp->ctx = libxl__gc_owner(gc); @@ -449,7 +448,7 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp) } else if (ret < 0) { if (errno == EINTR) continue; - LIBXL__LOG_ERRNO(qmp->ctx, LIBXL__LOG_ERROR, "Select error"); + LOGE(ERROR, "Select error"); return -1; } @@ -458,7 +457,7 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp) LOG(ERROR, "Unexpected end of socket"); return -1; } else if (rd < 0) { - LIBXL__LOG_ERRNO(qmp->ctx, LIBXL__LOG_ERROR, "Socket read error"); + LOGE(ERROR, "Socket read error"); return rd; } @@ -545,8 +544,7 @@ static char *qmp_send_prepare(libxl__gc *gc, libxl__qmp_handler *qmp, elm = malloc(sizeof (callback_id_pair)); if (elm == NULL) { - LIBXL__LOG_ERRNO(qmp->ctx, LIBXL__LOG_ERROR, - "Failed to allocate a QMP callback"); + LOGE(ERROR, "Failed to allocate a QMP callback"); goto out; } elm->id = qmp->last_id_used; @@ -704,7 +702,7 @@ libxl__qmp_handler *libxl__qmp_initialize(libxl__gc *gc, uint32_t domid) qmp_socket = libxl__sprintf(gc, "%s/qmp-libxl-%d", libxl__run_dir_path(), domid); if ((ret = qmp_open(qmp, qmp_socket, QMP_SOCKET_CONNECT_TIMEOUT)) < 0) { - LIBXL__LOG_ERRNO(qmp->ctx, LIBXL__LOG_ERROR, "Connection error"); + LOGE(ERROR, "Connection error"); qmp_free_handler(qmp); return NULL; } @@ -736,16 +734,13 @@ void libxl__qmp_close(libxl__qmp_handler *qmp) void libxl__qmp_cleanup(libxl__gc *gc, uint32_t domid) { - libxl_ctx *ctx = libxl__gc_owner(gc); char *qmp_socket; qmp_socket = libxl__sprintf(gc, "%s/qmp-libxl-%d", libxl__run_dir_path(), domid); if (unlink(qmp_socket) == -1) { if (errno != ENOENT) { - LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, - "Failed to remove QMP socket file %s", - qmp_socket); + LOGE(ERROR, "Failed to remove QMP socket file %s", qmp_socket); } } } -- 1.8.1.2
Kelley Nielsen
2013-Nov-11 15:50 UTC
[PATCH OPW 2/7] libxl: replace last invocation of LIBXL__LOG_ERROR with LOGE
Code cleanup -- no functional changes One invocation of the macro LIBXL__LOG_ERROR remains in libxl_qmp.c. It is in a function, register_serials_chardev_callback(), that does not have a local libxl__gc gc* in the original code. Since one was added in a previous patch, change this last invocation to an invocation of LOGE. Note: This patch depends on the patch "change remaining LIBXL_LOG to LOG in libxl_qmp.c", which was submitted at 04:05, November 10, 2013. Suggested-by: Anthony PERARD <anthony.perard@citrix.com> Signed-off-by: Kelley Nielsen <kelleynnn@gmail.com> Acked-by: Ian Campbell <ian.campbell@citrix.com> --- tools/libxl/libxl_qmp.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c index a181383..c6597d5 100644 --- a/tools/libxl/libxl_qmp.c +++ b/tools/libxl/libxl_qmp.c @@ -144,9 +144,8 @@ static int register_serials_chardev_callback(libxl__qmp_handler *qmp, } ret = store_serial_port_info(qmp, chardev, port_number); if (ret) { - LIBXL__LOG_ERRNO(qmp->ctx, LIBXL__LOG_ERROR, - "Failed to store serial port information" - " in xenstore"); + LOGE(ERROR, "Failed to store serial port information" + " in xenstore"); goto out; } } -- 1.8.1.2
Kelley Nielsen
2013-Nov-11 15:50 UTC
[PATCH OPW 3/7] libxl: use GCSPRINTF in place of libxl_sprintf() in libxl_qmp.c
Code cleanup -- no functional changes The convenience macro GCSPRINTF has been written to be used in place of libxl_sprintf. Change all calls to libxl_sprintf() in libxl_qmp.c to invocations of the new macro. Suggested-by: Anthony PERARD <anthony.perard@citrix.com> Signed-off-by: Kelley Nielsen <kelleynnn@gmail.com> --- tools/libxl/libxl_qmp.c | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c index c6597d5..b0ec2e8 100644 --- a/tools/libxl/libxl_qmp.c +++ b/tools/libxl/libxl_qmp.c @@ -101,7 +101,7 @@ static int store_serial_port_info(libxl__qmp_handler *qmp, } path = libxl__xs_get_dompath(gc, qmp->domid); - path = libxl__sprintf(gc, "%s/serial/%d/tty", path, port); + path = GCSPRINTF("%s/serial/%d/tty", path, port); ret = libxl__xs_write(gc, XBT_NULL, path, "%s", chardev + 4); @@ -162,7 +162,7 @@ static int qmp_write_domain_console_item(libxl__gc *gc, int domid, char *path; path = libxl__xs_get_dompath(gc, domid); - path = libxl__sprintf(gc, "%s/console/%s", path, item); + path = GCSPRINTF("%s/console/%s", path, item); return libxl__xs_write(gc, XBT_NULL, path, "%s", value); } @@ -683,8 +683,7 @@ static void qmp_parameters_add_integer(libxl__gc *gc, } #define QMP_PARAMETERS_SPRINTF(args, name, format, ...) \ - qmp_parameters_add_string(gc, args, name, \ - libxl__sprintf(gc, format, __VA_ARGS__)) + qmp_parameters_add_string(gc, args, name, GCSPRINTF(format, __VA_ARGS__)) /* * API @@ -698,8 +697,7 @@ libxl__qmp_handler *libxl__qmp_initialize(libxl__gc *gc, uint32_t domid) qmp = qmp_init_handler(gc, domid); - qmp_socket = libxl__sprintf(gc, "%s/qmp-libxl-%d", - libxl__run_dir_path(), domid); + qmp_socket = GCSPRINTF("%s/qmp-libxl-%d", libxl__run_dir_path(), domid); if ((ret = qmp_open(qmp, qmp_socket, QMP_SOCKET_CONNECT_TIMEOUT)) < 0) { LOGE(ERROR, "Connection error"); qmp_free_handler(qmp); @@ -735,8 +733,7 @@ void libxl__qmp_cleanup(libxl__gc *gc, uint32_t domid) { char *qmp_socket; - qmp_socket = libxl__sprintf(gc, "%s/qmp-libxl-%d", - libxl__run_dir_path(), domid); + qmp_socket = GCSPRINTF("%s/qmp-libxl-%d", libxl__run_dir_path(), domid); if (unlink(qmp_socket) == -1) { if (errno != ENOENT) { LOGE(ERROR, "Failed to remove QMP socket file %s", qmp_socket); @@ -765,8 +762,8 @@ static int pci_add_callback(libxl__qmp_handler *qmp, const libxl__json_object *bus = NULL; GC_INIT(qmp->ctx); int i, j, rc = -1; - char *asked_id = libxl__sprintf(gc, PCI_PT_QDEV_ID, - pcidev->bus, pcidev->dev, pcidev->func); + char *asked_id = GCSPRINTF(PCI_PT_QDEV_ID, + pcidev->bus, pcidev->dev, pcidev->func); for (i = 0; (bus = libxl__json_array_get(response, i)); i++) { const libxl__json_object *devices = NULL; @@ -834,8 +831,8 @@ int libxl__qmp_pci_add(libxl__gc *gc, int domid, libxl_device_pci *pcidev) if (!qmp) return -1; - hostaddr = libxl__sprintf(gc, "%04x:%02x:%02x.%01x", pcidev->domain, - pcidev->bus, pcidev->dev, pcidev->func); + hostaddr = GCSPRINTF("%04x:%02x:%02x.%01x", pcidev->domain, + pcidev->bus, pcidev->dev, pcidev->func); if (!hostaddr) return -1; @@ -871,8 +868,7 @@ int libxl__qmp_pci_del(libxl__gc *gc, int domid, libxl_device_pci *pcidev) { char *id = NULL; - id = libxl__sprintf(gc, PCI_PT_QDEV_ID, - pcidev->bus, pcidev->dev, pcidev->func); + id = GCSPRINTF(PCI_PT_QDEV_ID, pcidev->bus, pcidev->dev, pcidev->func); return qmp_device_del(gc, domid, id); } -- 1.8.1.2
Kelley Nielsen
2013-Nov-11 15:50 UTC
[PATCH OPW 4/7] libxl: use GCSPRINTF instead of libxl__sprintf
Code cleanup - no functional changes The convenience macro GCSPRINTF has been written to be used in place of libxl__sprintf(). Replace all calls to libxl__sprintf() in libxl_utils.c with invocations of the new macro. Suggested-by: Ian Campbell <Ian.Campbell@citrix.com> Signed-off-by: Kelley Nielsen <kelleynnn@gmail.com> Acked-by: Ian Campbell <ian.campbell@citrix.com> --- tools/libxl/libxl_utils.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c index 682f874..509d648 100644 --- a/tools/libxl/libxl_utils.c +++ b/tools/libxl/libxl_utils.c @@ -173,8 +173,8 @@ int libxl_get_stubdom_id(libxl_ctx *ctx, int guest_domid) int ret; stubdom_id_s = libxl__xs_read(gc, XBT_NULL, - libxl__sprintf(gc, "%s/image/device-model-domid", - libxl__xs_get_dompath(gc, guest_domid))); + GCSPRINTF("%s/image/device-model-domid", + libxl__xs_get_dompath(gc, guest_domid))); if (stubdom_id_s) ret = atoi(stubdom_id_s); else @@ -190,7 +190,7 @@ int libxl_is_stubdom(libxl_ctx *ctx, uint32_t domid, uint32_t *target_domid) uint32_t value; int ret = 0; - target = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/target", libxl__xs_get_dompath(gc, domid))); + target = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/target", libxl__xs_get_dompath(gc, domid))); if (!target) goto out; value = strtol(target, &endptr, 10); @@ -227,20 +227,20 @@ int libxl_create_logfile(libxl_ctx *ctx, const char *name, char **full_name) char *logfile, *logfile_new; int i, rc; - logfile = libxl__sprintf(gc, "/var/log/xen/%s.log", name); + logfile = GCSPRINTF("/var/log/xen/%s.log", name); if (stat(logfile, &stat_buf) == 0) { /* file exists, rotate */ - logfile = libxl__sprintf(gc, "/var/log/xen/%s.log.10", name); + logfile = GCSPRINTF("/var/log/xen/%s.log.10", name); unlink(logfile); for (i = 9; i > 0; i--) { - logfile = libxl__sprintf(gc, "/var/log/xen/%s.log.%d", name, i); - logfile_new = libxl__sprintf(gc, "/var/log/xen/%s.log.%d", name, i + 1); + logfile = GCSPRINTF("/var/log/xen/%s.log.%d", name, i); + logfile_new = GCSPRINTF("/var/log/xen/%s.log.%d", name, i + 1); rc = logrename(gc, logfile, logfile_new); if (rc) goto out; } - logfile = libxl__sprintf(gc, "/var/log/xen/%s.log", name); - logfile_new = libxl__sprintf(gc, "/var/log/xen/%s.log.1", name); + logfile = GCSPRINTF("/var/log/xen/%s.log", name); + logfile_new = GCSPRINTF("/var/log/xen/%s.log.1", name); rc = logrename(gc, logfile, logfile_new); if (rc) -- 1.8.1.2
Kelley Nielsen
2013-Nov-11 15:50 UTC
[PATCH OPW 5/7] libxl: use LOGE instead of LIBXL__LOG_ERRNO in libxl_utils.c
Code cleanup - no functional changes The convenience macro LOGE has been written to take the place of LIBXL__LOG_ERRNO. LOGE depends on the existence of a local libgl__gc *gc. Replace two invocations of LIBXL__LOG_ERRNO, which are in functions that already have a libxl__gc *gc present, to invocations of the new macro. Suggested-by: Ian Campbell <Ian.Campbell@citrix.com> Signed-off-by: Kelley Nielsen <kelleynnn@gmail.com> --- tools/libxl/libxl_utils.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c index 509d648..03ba6d8 100644 --- a/tools/libxl/libxl_utils.c +++ b/tools/libxl/libxl_utils.c @@ -206,15 +206,14 @@ out: static int logrename(libxl__gc *gc, const char *old, const char *new) { - libxl_ctx *ctx = libxl__gc_owner(gc); int r; r = rename(old, new); if (r) { if (errno == ENOENT) return 0; /* ok */ - LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to rotate logfile - could not" - " rename %s to %s", old, new); + LOGE(ERROR, "failed to rotate logfile - " + "could not rename %s to %s", old, new); return ERROR_FAIL; } return 0; @@ -247,9 +246,9 @@ int libxl_create_logfile(libxl_ctx *ctx, const char *name, char **full_name) goto out; } else { if (errno != ENOENT) - LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_WARNING, "problem checking existence of" - " logfile %s, which might have needed to be rotated", - name); + LOGE(WARN, "problem checking existence of logfile %s, " + "which might have needed to be rotated", + name); } *full_name = strdup(logfile); rc = 0; -- 1.8.1.2
Kelley Nielsen
2013-Nov-11 15:50 UTC
[PATCH OPW 6/7] libxl: use LOG and LOGE instead of LIBXL__LOG* in libxl_utils.c
Code cleanup - no functional changes The convenience macros LOG and LOGE have been written to take the place of the old macros in the LIBXL__LOG* family. Replace the invocations of the old macros in the function libxl_read_file_contents() with invocations of the corresponding new ones. Create a local libxl__gc gc* for the new macros to use by invoking GC_INIT(ctx) at the top of the function, and clean it up by invoking GC_FREE at the two exit points. Suggested-by: Ian Campbell <Ian.Campbell@citrix.com> Signed-off-by: Kelley Nielsen <kelleynnn@gmail.com> Acked-by: Ian Campbell <ian.campbell@citrix.com> --- tools/libxl/libxl_utils.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c index 03ba6d8..5ed9e1b 100644 --- a/tools/libxl/libxl_utils.c +++ b/tools/libxl/libxl_utils.c @@ -289,6 +289,7 @@ out: int libxl_read_file_contents(libxl_ctx *ctx, const char *filename, void **data_r, int *datalen_r) { + GC_INIT(ctx); FILE *f = 0; uint8_t *data = 0; int datalen = 0; @@ -299,23 +300,23 @@ int libxl_read_file_contents(libxl_ctx *ctx, const char *filename, f = fopen(filename, "r"); if (!f) { if (errno == ENOENT) return ENOENT; - LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to open %s", filename); + LOGE(ERROR, "failed to open %s", filename); goto xe; } if (fstat(fileno(f), &stab)) { - LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to fstat %s", filename); + LOGE(ERROR, "failed to fstat %s", filename); goto xe; } if (!S_ISREG(stab.st_mode)) { - LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "%s is not a plain file", filename); + LOGE(ERROR, "%s is not a plain file", filename); errno = ENOTTY; goto xe; } if (stab.st_size > INT_MAX) { - LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "file %s is far too large", filename); + LOG(ERROR, "file %s is far too large", filename); errno = EFBIG; goto xe; } @@ -329,10 +330,10 @@ int libxl_read_file_contents(libxl_ctx *ctx, const char *filename, rs = fread(data, 1, datalen, f); if (rs != datalen) { if (ferror(f)) - LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to read %s", filename); + LOGE(ERROR, "failed to read %s", filename); else if (feof(f)) - LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "%s changed size while we" - " were reading it", filename); + LOG(ERROR, "%s changed size while we were reading it", + filename); else abort(); goto xe; @@ -341,16 +342,18 @@ int libxl_read_file_contents(libxl_ctx *ctx, const char *filename, if (fclose(f)) { f = 0; - LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to close %s", filename); + LOGE(ERROR, "failed to close %s", filename); goto xe; } if (data_r) *data_r = data; if (datalen_r) *datalen_r = datalen; + GC_FREE; return 0; xe: + GC_FREE; e = errno; assert(e != ENOENT); if (f) fclose(f); -- 1.8.1.2
Kelley Nielsen
2013-Nov-11 15:50 UTC
[PATCH OPW 7/7] libxl: use LOG instead of LIBXL__LOG in libxl_utils.c
To conform to the new coding style, replace the invocation of LIBXL__LOG in the function libxl_pipe() in the file libxl_utils.c with an invocation of LOG. Create a local libxl__gc gc* for LOG to use by invoking GC_INIT(ctx) at the top of the function, and clean it up by invoking GC_FREE at the exit. Create a variable, ret, to consolidate exits in one place and avoid invoking GC_FREE twice. Suggested-by: Ian Campbell <Ian.Campbell@citrix.com> Signed-off-by: Kelley Nielsen <kelleynnn@gmail.com> Acked-by: Ian Campbell <ian.campbell@citrix.com> --- tools/libxl/libxl_utils.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c index 5ed9e1b..4c5f754 100644 --- a/tools/libxl/libxl_utils.c +++ b/tools/libxl/libxl_utils.c @@ -477,11 +477,14 @@ int libxl__remove_directory(libxl__gc *gc, const char *dirpath) int libxl_pipe(libxl_ctx *ctx, int pipes[2]) { + GC_INIT(ctx); + int ret = 0; if (pipe(pipes) < 0) { - LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Failed to create a pipe"); - return -1; + LOG(ERROR, "Failed to create a pipe"); + ret = -1; } - return 0; + GC_FREE; + return ret; } int libxl_uuid_to_device_vtpm(libxl_ctx *ctx, uint32_t domid, -- 1.8.1.2
Ian Campbell
2013-Nov-11 16:56 UTC
Re: [PATCH OPW 1/7] libxl: Use new macro LOGE() in libxl_qmp.c
On Mon, 2013-11-11 at 07:50 -0800, Kelley Nielsen wrote:> Code cleanup -- no functional changes > > Coding style has recently been changed for libxl. The convenience > macro LOGE() has been introduced, and invocations of the old macro > LIBXL__LOG_ERROR() are to be replaced with it. Change all occurences > of the old macro (in functions that have a local libxl_gc *gc) except > the one in register_serials_chardev_callback() to the new one. (This > function lacks a local libxl__gc *gc, which LOGE() requires.) > > Suggested-by: Anthony PERARD <anthony.perard@citrix.com> > Signed-off-by: Kelley Nielsen <kelleynnn@gmail.com>Acked-by: Ian Campbell <ian.campbell@citrix.com>
Ian Campbell
2013-Nov-11 16:58 UTC
Re: [PATCH OPW 3/7] libxl: use GCSPRINTF in place of libxl_sprintf() in libxl_qmp.c
On Mon, 2013-11-11 at 07:50 -0800, Kelley Nielsen wrote:> Code cleanup -- no functional changes > > The convenience macro GCSPRINTF has been written to be used in place of > libxl_sprintf. Change all calls to libxl_sprintf() in libxl_qmp.c to > invocations of the new macro. > > Suggested-by: Anthony PERARD <anthony.perard@citrix.com> > Signed-off-by: Kelley Nielsen <kelleynnn@gmail.com>Acked-by: Ian Campbell <ian.campbell@citrix.com>