* [PATCH 2/4] libxl_get_scheduler() cannot return ERROR_FAIL
2014-09-13 21:10 [PATCH 1/4] Use proper header guard for <ufs/ufs.h> Marcin Cieslak
@ 2014-09-13 21:10 ` Marcin Cieslak
2014-09-16 16:27 ` Ian Campbell
2014-09-13 21:10 ` [PATCH 3/4] libxl: shutdown_reason cannot be unsigned Marcin Cieslak
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Marcin Cieslak @ 2014-09-13 21:10 UTC (permalink / raw)
To: xen-devel; +Cc: Marcin Cieslak
ERROR_FAIL (-3) is not a proper value for
libxl_scheduler enum.
Use LIBXL_SCHEDULER_UNKNOWN (0) instead.
Clang complains otherwise:
xl_cmdimpl.c:4824:44: error: comparison of unsigned enum expression < 0 is always false [-Werror,-Wtautological-compare]
if ((sched = libxl_get_scheduler(ctx)) < 0) {
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~
xl_cmdimpl.c:6705:48: error: comparison of unsigned enum expression < 0 is always false [-Werror,-Wtautological-compare]
if ((sched = libxl_get_scheduler(ctx)) < 0) {
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~
C99 does not guarantee that enums are ints (they
can be unsigned).
---
tools/libxl/libxl.c | 2 +-
tools/libxl/xl_cmdimpl.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index ad3495a..785a1e7 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -4957,7 +4957,7 @@ libxl_scheduler libxl_get_scheduler(libxl_ctx *ctx)
if ((ret = xc_sched_id(ctx->xch, (int *)&sched)) != 0) {
LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info list");
- return ERROR_FAIL;
+ return LIBXL_SCHEDULER_UNKNOWN;
}
return sched;
}
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 8a38077..86daf8e 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -4821,7 +4821,7 @@ static void output_xeninfo(void)
return;
}
- if ((sched = libxl_get_scheduler(ctx)) < 0) {
+ if ((sched = libxl_get_scheduler(ctx)) == LIBXL_SCHEDULER_UNKNOWN) {
fprintf(stderr, "get_scheduler sysctl failed.\n");
return;
}
@@ -6702,7 +6702,7 @@ int main_cpupoolcreate(int argc, char **argv)
goto out_cfg;
}
} else {
- if ((sched = libxl_get_scheduler(ctx)) < 0) {
+ if ((sched = libxl_get_scheduler(ctx)) == LIBXL_SCHEDULER_UNKNOWN) {
fprintf(stderr, "get_scheduler sysctl failed.\n");
goto out_cfg;
}
--
2.0.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 2/4] libxl_get_scheduler() cannot return ERROR_FAIL
2014-09-13 21:10 ` [PATCH 2/4] libxl_get_scheduler() cannot return ERROR_FAIL Marcin Cieslak
@ 2014-09-16 16:27 ` Ian Campbell
0 siblings, 0 replies; 9+ messages in thread
From: Ian Campbell @ 2014-09-16 16:27 UTC (permalink / raw)
To: Marcin Cieslak; +Cc: xen-devel, Julien Grall
On Sat, 2014-09-13 at 21:10 +0000, Marcin Cieslak wrote:
> ERROR_FAIL (-3) is not a proper value for
> libxl_scheduler enum.
> Use LIBXL_SCHEDULER_UNKNOWN (0) instead.
This came up before, see [0,1] we decided instead that this function
should return an int, I think Julien simply hasn't found the time to
revisit that after the comments on the v2 patch.
[0] http://lists.xen.org/archives/html/xen-devel/2014-03/msg02476.html
[1] http://lists.xen.org/archives/html/xen-devel/2014-03/msg02759.html
>
> Clang complains otherwise:
>
> xl_cmdimpl.c:4824:44: error: comparison of unsigned enum expression < 0 is always false [-Werror,-Wtautological-compare]
> if ((sched = libxl_get_scheduler(ctx)) < 0) {
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~
> xl_cmdimpl.c:6705:48: error: comparison of unsigned enum expression < 0 is always false [-Werror,-Wtautological-compare]
> if ((sched = libxl_get_scheduler(ctx)) < 0) {
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~
>
> C99 does not guarantee that enums are ints (they
> can be unsigned).
> ---
> tools/libxl/libxl.c | 2 +-
> tools/libxl/xl_cmdimpl.c | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index ad3495a..785a1e7 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -4957,7 +4957,7 @@ libxl_scheduler libxl_get_scheduler(libxl_ctx *ctx)
>
> if ((ret = xc_sched_id(ctx->xch, (int *)&sched)) != 0) {
> LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info list");
> - return ERROR_FAIL;
> + return LIBXL_SCHEDULER_UNKNOWN;
> }
> return sched;
> }
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 8a38077..86daf8e 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -4821,7 +4821,7 @@ static void output_xeninfo(void)
> return;
> }
>
> - if ((sched = libxl_get_scheduler(ctx)) < 0) {
> + if ((sched = libxl_get_scheduler(ctx)) == LIBXL_SCHEDULER_UNKNOWN) {
> fprintf(stderr, "get_scheduler sysctl failed.\n");
> return;
> }
> @@ -6702,7 +6702,7 @@ int main_cpupoolcreate(int argc, char **argv)
> goto out_cfg;
> }
> } else {
> - if ((sched = libxl_get_scheduler(ctx)) < 0) {
> + if ((sched = libxl_get_scheduler(ctx)) == LIBXL_SCHEDULER_UNKNOWN) {
> fprintf(stderr, "get_scheduler sysctl failed.\n");
> goto out_cfg;
> }
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/4] libxl: shutdown_reason cannot be unsigned
2014-09-13 21:10 [PATCH 1/4] Use proper header guard for <ufs/ufs.h> Marcin Cieslak
2014-09-13 21:10 ` [PATCH 2/4] libxl_get_scheduler() cannot return ERROR_FAIL Marcin Cieslak
@ 2014-09-13 21:10 ` Marcin Cieslak
2014-09-16 18:43 ` Julien Grall
2014-09-13 21:10 ` [PATCH 4/4] libxl: Use yajl_gen_status_ok where appropriate Marcin Cieslak
2014-09-16 16:23 ` [PATCH 1/4] Use proper header guard for <ufs/ufs.h> Ian Campbell
3 siblings, 1 reply; 9+ messages in thread
From: Marcin Cieslak @ 2014-09-13 21:10 UTC (permalink / raw)
To: xen-devel; +Cc: Marcin Cieslak
Shutdown reason may be negative, so don't
cast it onto (unsigned int).
Clang complains:
xl_cmdimpl.c:3313:34: error: comparison of unsigned expression >= 0 is always true [-Werror,-Wtautological-compare]
(shutdown_reason >= 0 &&
~~~~~~~~~~~~~~~ ^ ~
---
tools/libxl/xl_cmdimpl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 86daf8e..f9ca22a 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -3297,7 +3297,7 @@ static void list_domains(int verbose, int context, int claim, int numa,
printf("\n");
for (i = 0; i < nb_domain; i++) {
char *domname;
- unsigned shutdown_reason;
+ int shutdown_reason;
domname = libxl_domid_to_name(ctx, info[i].domid);
shutdown_reason = info[i].shutdown ? info[i].shutdown_reason : 0;
printf("%-40s %5d %5lu %5d %c%c%c%c%c%c %8.1f",
--
2.0.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 3/4] libxl: shutdown_reason cannot be unsigned
2014-09-13 21:10 ` [PATCH 3/4] libxl: shutdown_reason cannot be unsigned Marcin Cieslak
@ 2014-09-16 18:43 ` Julien Grall
0 siblings, 0 replies; 9+ messages in thread
From: Julien Grall @ 2014-09-16 18:43 UTC (permalink / raw)
To: Marcin Cieslak, xen-devel, Ian Campbell, xen.org
Hello Marcin,
On 13/09/14 14:10, Marcin Cieslak wrote:
> Shutdown reason may be negative, so don't
> cast it onto (unsigned int).
If you are looking the place where the variable is assigned,
info[i].shutdown_reason is typed as an unsigned int.
So changing type doesn't look like the right solution. I've sent a few
months ago a patch to drop "shutdown_reason >= 0" check [1].
It was acked, but I hadn't had time to resent this series properly.
Mainly because I got other errors around with clang.
Regards,
[1] https://patches.linaro.org/27074/
> Clang complains:
>
> xl_cmdimpl.c:3313:34: error: comparison of unsigned expression >= 0 is always true [-Werror,-Wtautological-compare]
> (shutdown_reason >= 0 &&
> ~~~~~~~~~~~~~~~ ^ ~
> ---
> tools/libxl/xl_cmdimpl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 86daf8e..f9ca22a 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -3297,7 +3297,7 @@ static void list_domains(int verbose, int context, int claim, int numa,
> printf("\n");
> for (i = 0; i < nb_domain; i++) {
> char *domname;
> - unsigned shutdown_reason;
> + int shutdown_reason;
> domname = libxl_domid_to_name(ctx, info[i].domid);
> shutdown_reason = info[i].shutdown ? info[i].shutdown_reason : 0;
> printf("%-40s %5d %5lu %5d %c%c%c%c%c%c %8.1f",
>
--
Julien Grall
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 4/4] libxl: Use yajl_gen_status_ok where appropriate
2014-09-13 21:10 [PATCH 1/4] Use proper header guard for <ufs/ufs.h> Marcin Cieslak
2014-09-13 21:10 ` [PATCH 2/4] libxl_get_scheduler() cannot return ERROR_FAIL Marcin Cieslak
2014-09-13 21:10 ` [PATCH 3/4] libxl: shutdown_reason cannot be unsigned Marcin Cieslak
@ 2014-09-13 21:10 ` Marcin Cieslak
2014-09-16 16:29 ` Ian Campbell
2014-09-16 16:23 ` [PATCH 1/4] Use proper header guard for <ufs/ufs.h> Ian Campbell
3 siblings, 1 reply; 9+ messages in thread
From: Marcin Cieslak @ 2014-09-13 21:10 UTC (permalink / raw)
To: xen-devel; +Cc: Marcin Cieslak
---
tools/libxl/libxl_internal.h | 2 +-
tools/libxl/libxl_json.c | 10 +++++-----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 03e9978..f006719 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1772,7 +1772,7 @@ libxl__json_map_node *libxl__json_map_node_get(const libxl__json_object *o,
_hidden const libxl__json_object *libxl__json_map_get(const char *key,
const libxl__json_object *o,
libxl__json_node_type expected_type);
-_hidden yajl_status libxl__json_object_to_yajl_gen(libxl__gc *gc_opt,
+_hidden yajl_gen_status libxl__json_object_to_yajl_gen(libxl__gc *gc_opt,
yajl_gen hand,
libxl__json_object *param);
_hidden void libxl__json_object_free(libxl__gc *gc_opt,
diff --git a/tools/libxl/libxl_json.c b/tools/libxl/libxl_json.c
index ceb014a..1481140 100644
--- a/tools/libxl/libxl_json.c
+++ b/tools/libxl/libxl_json.c
@@ -610,12 +610,12 @@ const libxl__json_object *libxl__json_map_get(const char *key,
return NULL;
}
-yajl_status libxl__json_object_to_yajl_gen(libxl__gc *gc,
+yajl_gen_status libxl__json_object_to_yajl_gen(libxl__gc *gc,
yajl_gen hand,
libxl__json_object *obj)
{
int idx = 0;
- yajl_status rc;
+ yajl_gen_status rc;
switch (obj->type) {
case JSON_NULL:
@@ -634,17 +634,17 @@ yajl_status libxl__json_object_to_yajl_gen(libxl__gc *gc,
libxl__json_map_node *node = NULL;
rc = yajl_gen_map_open(hand);
- if (rc != yajl_status_ok)
+ if (rc != yajl_gen_status_ok)
return rc;
for (idx = 0; idx < obj->u.map->count; idx++) {
if (flexarray_get(obj->u.map, idx, (void**)&node) != 0)
break;
rc = libxl__yajl_gen_asciiz(hand, node->map_key);
- if (rc != yajl_status_ok)
+ if (rc != yajl_gen_status_ok)
return rc;
rc = libxl__json_object_to_yajl_gen(gc, hand, node->obj);
- if (rc != yajl_status_ok)
+ if (rc != yajl_gen_status_ok)
return rc;
}
return yajl_gen_map_close(hand);
--
2.0.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 1/4] Use proper header guard for <ufs/ufs.h>
2014-09-13 21:10 [PATCH 1/4] Use proper header guard for <ufs/ufs.h> Marcin Cieslak
` (2 preceding siblings ...)
2014-09-13 21:10 ` [PATCH 4/4] libxl: Use yajl_gen_status_ok where appropriate Marcin Cieslak
@ 2014-09-16 16:23 ` Ian Campbell
2014-09-16 16:28 ` Ian Campbell
3 siblings, 1 reply; 9+ messages in thread
From: Ian Campbell @ 2014-09-16 16:23 UTC (permalink / raw)
To: Marcin Cieslak; +Cc: xen-devel
On Sat, 2014-09-13 at 21:10 +0000, Marcin Cieslak wrote:
I didn't spot this on the first patch I commented on but all patches
accepted into Xen need to have a Signed-off-by. Please see
http://wiki.xen.org/wiki/Submitting_Xen_Patches#Signing_off_a_patch.
Ian.
^ permalink raw reply [flat|nested] 9+ messages in thread