| Message ID | 1461139444-12342-8-git-send-email-paulinaszubarczyk@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Apr 20, Paulina Szubarczyk wrote: > In accordance with CODING_SYTLE: > - Use 'r' for return values to functions whose return values are a > different error space (like xc_tmem_control, xc_tmem_auth) > > libxc functions are supposed to, on failure, set errno and alwas return -1 > which is the value stored in 'r', therfore use LOG() instead LOGEV() > with the 'r' value since the error code is always -1. Shouldnt in this case 'LOGEV' be replaced with 'LOGE' to get errno printed? Plain 'LOG' does not seem to print errno. Olaf
On Wed, Apr 20, 2016 at 11:04:06PM +0200, Olaf Hering wrote: > On Wed, Apr 20, Paulina Szubarczyk wrote: > > > In accordance with CODING_SYTLE: > > - Use 'r' for return values to functions whose return values are a > > different error space (like xc_tmem_control, xc_tmem_auth) > > > > libxc functions are supposed to, on failure, set errno and alwas return -1 > > which is the value stored in 'r', therfore use LOG() instead LOGEV() > > with the 'r' value since the error code is always -1. > > Shouldnt in this case 'LOGEV' be replaced with 'LOGE' to get errno > printed? Plain 'LOG' does not seem to print errno. > Agreed. We should use LOGEV here. Wei. > Olaf
On Wed, 2016-04-27 at 15:31 +0100, Wei Liu wrote: > On Wed, Apr 20, 2016 at 11:04:06PM +0200, Olaf Hering wrote: > > > > On Wed, Apr 20, Paulina Szubarczyk wrote: > > > > > > > > In accordance with CODING_SYTLE: > > > - Use 'r' for return values to functions whose return values are > > > a > > > different error space (like xc_tmem_control, xc_tmem_auth) > > > > > > libxc functions are supposed to, on failure, set errno and alwas > > > return -1 > > > which is the value stored in 'r', therfore use LOG() instead > > > LOGEV() > > > with the 'r' value since the error code is always -1. > > Shouldnt in this case 'LOGEV' be replaced with 'LOGE' to get errno > > printed? Plain 'LOG' does not seem to print errno. > > > Agreed. We should use LOGEV here. > You mean LOGE, don't you? :-) I think it was me that suggested dropping LOGEV in favour of LOG, and I agree that I should have suggested LOGE. Sorry for that. TBF, I seem to find both cases in libxl source code (i.e., we use both LOG and LOGE to log error after an xc_* call), but I agree that, since libxc sets errno, LOGE is the better, and we should try to converge to always using that one. Regards, Dario
On Wed, Apr 27, 2016 at 06:53:35PM +0200, Dario Faggioli wrote: > On Wed, 2016-04-27 at 15:31 +0100, Wei Liu wrote: > > On Wed, Apr 20, 2016 at 11:04:06PM +0200, Olaf Hering wrote: > > > > > > On Wed, Apr 20, Paulina Szubarczyk wrote: > > > > > > > > > > > In accordance with CODING_SYTLE: > > > > - Use 'r' for return values to functions whose return values are > > > > a > > > > different error space (like xc_tmem_control, xc_tmem_auth) > > > > > > > > libxc functions are supposed to, on failure, set errno and alwas > > > > return -1 > > > > which is the value stored in 'r', therfore use LOG() instead > > > > LOGEV() > > > > with the 'r' value since the error code is always -1. > > > Shouldnt in this case 'LOGEV' be replaced with 'LOGE' to get errno > > > printed? Plain 'LOG' does not seem to print errno. > > > > > Agreed. We should use LOGEV here. > > > You mean LOGE, don't you? :-) > Yes, LOGE. Sorry. > I think it was me that suggested dropping LOGEV in favour of LOG, and I > agree that I should have suggested LOGE. Sorry for that. > > TBF, I seem to find both cases in libxl source code (i.e., we use both > LOG and LOGE to log error after an xc_* call), but I agree that, since > libxc sets errno, LOGE is the better, and we should try to converge to > always using that one. > Yes. Wei. > Regards, > Dario > -- > <<This happens because I choose it to happen!>> (Raistlin Majere) > ----------------------------------------------------------------- > Dario Faggioli, Ph.D, http://about.me/dario.faggioli > Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) >
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c index 9130f74..02b6164 100644 --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -6597,14 +6597,14 @@ uint32_t libxl_vm_get_start_time(libxl_ctx *ctx, uint32_t domid) char *libxl_tmem_list(libxl_ctx *ctx, uint32_t domid, int use_long) { - int rc; + int r; char _buf[32768]; GC_INIT(ctx); - rc = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_LIST, domid, 32768, use_long, - _buf); - if (rc < 0) { - LOGEV(ERROR, rc, "Can not get tmem list"); + r = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_LIST, domid, 32768, + use_long, _buf); + if (r < 0) { + LOG(ERROR, "Can not get tmem list"); GC_FREE; return NULL; } @@ -6615,34 +6615,38 @@ char *libxl_tmem_list(libxl_ctx *ctx, uint32_t domid, int use_long) int libxl_tmem_freeze(libxl_ctx *ctx, uint32_t domid) { - int rc; + int r, rc; GC_INIT(ctx); - rc = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_FREEZE, domid, 0, 0, - NULL); - if (rc < 0) { - LOGEV(ERROR, rc, "Can not freeze tmem pools"); - GC_FREE; - return ERROR_FAIL; + r = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_FREEZE, domid, 0, 0, + NULL); + if (r < 0) { + LOG(ERROR, "Can not freeze tmem pools"); + rc = ERROR_FAIL; + goto out; } + rc = 0; +out: GC_FREE; return rc; } int libxl_tmem_thaw(libxl_ctx *ctx, uint32_t domid) { - int rc; + int r, rc; GC_INIT(ctx); - rc = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_THAW, domid, 0, 0, - NULL); - if (rc < 0) { - LOGEV(ERROR, rc, "Can not thaw tmem pools"); - GC_FREE; - return ERROR_FAIL; + r = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_THAW, domid, 0, 0, + NULL); + if (r < 0) { + LOG(ERROR, "Can not thaw tmem pools"); + rc = ERROR_FAIL; + goto out; } + rc = 0; +out: GC_FREE; return rc; } @@ -6661,22 +6665,24 @@ static int32_t tmem_setop_from_string(char *set_name) int libxl_tmem_set(libxl_ctx *ctx, uint32_t domid, char* name, uint32_t set) { - int rc; + int r, rc; int32_t subop = tmem_setop_from_string(name); GC_INIT(ctx); if (subop == -1) { LOGEV(ERROR, -1, "Invalid set, valid sets are <weight|cap|compress>"); - GC_FREE; - return ERROR_INVAL; + rc = ERROR_INVAL; + goto out; } - rc = xc_tmem_control(ctx->xch, -1, subop, domid, set, 0, NULL); - if (rc < 0) { - LOGEV(ERROR, rc, "Can not set tmem %s", name); - GC_FREE; - return ERROR_FAIL; + r = xc_tmem_control(ctx->xch, -1, subop, domid, set, 0, NULL); + if (r < 0) { + LOG(ERROR, "Can not set tmem %s", name); + rc = ERROR_FAIL; + goto out; } + rc = 0; +out: GC_FREE; return rc; } @@ -6684,32 +6690,37 @@ int libxl_tmem_set(libxl_ctx *ctx, uint32_t domid, char* name, uint32_t set) int libxl_tmem_shared_auth(libxl_ctx *ctx, uint32_t domid, char* uuid, int auth) { - int rc; + int r, rc; GC_INIT(ctx); - rc = xc_tmem_auth(ctx->xch, domid, uuid, auth); - if (rc < 0) { - LOGEV(ERROR, rc, "Can not set tmem shared auth"); - GC_FREE; - return ERROR_FAIL; + r = xc_tmem_auth(ctx->xch, domid, uuid, auth); + if (r < 0) { + LOG(ERROR, "Can not set tmem shared auth"); + rc = ERROR_FAIL; + goto out; } + rc = 0; +out: GC_FREE; return rc; } int libxl_tmem_freeable(libxl_ctx *ctx) { - int rc; + int r, rc; GC_INIT(ctx); - rc = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_QUERY_FREEABLE_MB, -1, 0, 0, 0); - if (rc < 0) { - LOGEV(ERROR, rc, "Can not get tmem freeable memory"); - GC_FREE; - return ERROR_FAIL; + r = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_QUERY_FREEABLE_MB, + -1, 0, 0, 0); + if (r < 0) { + LOG(ERROR, "Can not get tmem freeable memory"); + rc = ERROR_FAIL; + goto out; } + rc = 0; +out: GC_FREE; return rc; }
In accordance with CODING_SYTLE: - Use 'r' for return values to functions whose return values are a different error space (like xc_tmem_control, xc_tmem_auth) libxc functions are supposed to, on failure, set errno and alwas return -1 which is the value stored in 'r', therfore use LOG() instead LOGEV() with the 'r' value since the error code is always -1. Signed-off-by: Paulina Szubarczyk <paulinaszubarczyk@gmail.com> --- Changed since v2: * corrected indentation * changed LOGEV()->LOG() --- tools/libxl/libxl.c | 89 ++++++++++++++++++++++++++++++----------------------- 1 file changed, 50 insertions(+), 39 deletions(-)