| Message ID | 1395272536-5157-1-git-send-email-julien.grall@linaro.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
Julien Grall writes ("[PATCH] tools/libxl: Correctly check if libxl_get_scheduler has failed"):
> libxl_get_scheduler will return an enum, therefore checking if the value
> is negative is wrong.
Gah. enums in C are so hateful.
> - if ((sched = libxl_get_scheduler(ctx)) < 0) {
> + if ((int)(sched = libxl_get_scheduler(ctx)) < 0) {
I'm afraid this isn't the right fix. The right fix is not to use the
enum type at all. libxl_get_scheduler, and the "sched" local, should
be ints.
Ian.
Hi Ian, On 03/20/2014 01:23 PM, Ian Jackson wrote: > Julien Grall writes ("[PATCH] tools/libxl: Correctly check if libxl_get_scheduler has failed"): >> libxl_get_scheduler will return an enum, therefore checking if the value >> is negative is wrong. > > Gah. enums in C are so hateful. > >> - if ((sched = libxl_get_scheduler(ctx)) < 0) { >> + if ((int)(sched = libxl_get_scheduler(ctx)) < 0) { > > I'm afraid this isn't the right fix. The right fix is not to use the > enum type at all. libxl_get_scheduler, and the "sched" local, should > be ints. Thanks. I will rework the patch. Regards,
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c index 8990020..8f6c411 100644 --- a/tools/libxl/xl_cmdimpl.c +++ b/tools/libxl/xl_cmdimpl.c @@ -4826,7 +4826,7 @@ static void output_xeninfo(void) return; } - if ((sched = libxl_get_scheduler(ctx)) < 0) { + if ((int)(sched = libxl_get_scheduler(ctx)) < 0) { fprintf(stderr, "get_scheduler sysctl failed.\n"); return; } @@ -6706,7 +6706,7 @@ int main_cpupoolcreate(int argc, char **argv) goto out_cfg; } } else { - if ((sched = libxl_get_scheduler(ctx)) < 0) { + if ((int)(sched = libxl_get_scheduler(ctx)) < 0) { fprintf(stderr, "get_scheduler sysctl failed.\n"); goto out_cfg; }
libxl_get_scheduler will return an enum, therefore checking if the value is negative is wrong. Both GCC and clang will never go to the error case. Spotted by clang: xl_cmdimpl.c:6709:48: error: comparison of unsigned enum expression < 0 is always false [-Werror,-Wtautological-compare] if ((sched = libxl_get_scheduler(ctx)) < 0) { ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~ Signed-off-by: Julien Grall <julien.grall@linaro.org> --- I'm not sure this is the right way to test if libxl_get_scheduler has failed. This small program should print ERROR, but on both clang and gcc it will print OK. #include <stdio.h> typedef enum libxl_error { ERROR_FAIL = -3, } libxl_error; typedef enum libxl_sched { SCHED_SCHED, } libxl_sched; libxl_sched f(void) { return ERROR_FAIL; } int main(void) { printf("f() = %d\n", f()); if ( f() < 0 ) printf("ERROR\n"); else printf("OK\n"); return 0; } --- tools/libxl/xl_cmdimpl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)