Fix build failure with gcc's -Werror=switch

PATCH New Dario Faggioli 3 participants 8 messages Jun 4, 2012 – Jun 6, 2012

lore

Share & tools

lore

Dario Faggioli

author

2012-06-04 17:23 UTC #0 lore raw

PATCH v5 1/1 Fix build failure with gcc's -Werror=switch

Hi guys,

This is another attempt of fixing that build failure with recent gcc-s properly (the one due to -Werror=switch enabled by default).

Quoted 1 line

From previous versions (namely, v4), I rewrote it as per IanJ's suggestion of

going for all the call sites of libxl__domain_type() and deal with the fact it can fail. This way, and thanks to the libxl__domain_build_info_setdefault() machinery, all the places where something different from LIBXL_DOMAIN_TYPE_PV or LIBXL_DOMAIN_TYPE_HVM is returned can just fail or abort.

Thanks and Regards,
Dario

Signature · 4 lines

<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

Dario Faggioli

author

2012-06-04 17:23 UTC #1 lore raw

PATCH v5 1/1 libxl: introduce LIBXL_DOMAIN_TYPE_INVALID

To avoid recent gcc complaining about:
libxl.c: In function ‘libxl_primary_console_exec’:
libxl.c:1233:9: error: case value ‘4294967295’ not in enumerated type ‘libxl_domain_type’ [-Werror=switch]

Also:
- have all the call sites of libxl__domain_type() return with error in case the function returns LIBXL_DOMAIN_TYPE_INVALID;
- adjust all other code segments where -Wswitch makes would claim that LIBXL_DOMAIN_TYPE_INVALID is not handled by adding a "default: abort();" clause.

Signed-off-by Dario Faggioli <[email protected]>

Signed-off-by Christoph Egger <[email protected]>

tools/libxl/libxl.c +17 -1

@@ -656,6 +656,13 @@ int libxl_domain_remus_start(libxl_ctx *
     libxl_domain_type type = libxl__domain_type(gc, domid);
     int rc = 0;
 
+    if (type == LIBXL_DOMAIN_TYPE_INVALID) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                   "invalid domain type for domain %d", domid);
+        rc = ERROR_INVAL;
+        goto remus_fail;
+    }
+
     if (info == NULL) {
         LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
                    "No remus_info structure supplied for domain %d", domid);

@@ -692,11 +699,20 @@ int libxl_domain_suspend(libxl_ctx *ctx,
     int debug = info != NULL && info->flags & XL_SUSPEND_DEBUG;
     int rc = 0;
 
+    if (type == LIBXL_DOMAIN_TYPE_INVALID) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                   "invalid domain type for domain %d", domid);
+        rc = ERROR_INVAL;
+        goto suspend_fail;
+    }
+
     rc = libxl__domain_suspend_common(gc, domid, fd, type, live, debug,
                                       /* No Remus */ NULL);
 
     if (!rc && type == LIBXL_DOMAIN_TYPE_HVM)
         rc = libxl__domain_save_device_model(gc, domid, fd);
+
+ suspend_fail:
     GC_FREE;
     return rc;
 }

@@ -1259,7 +1275,7 @@ int libxl_primary_console_exec(libxl_ctx
         case LIBXL_DOMAIN_TYPE_PV:
             rc = libxl_console_exec(ctx, domid_vm, 0, LIBXL_CONSOLE_TYPE_PV);
             break;
-        case -1:
+        case LIBXL_DOMAIN_TYPE_INVALID:
             LOG(ERROR,"unable to get domain type for domid=%"PRIu32,domid_vm);
             rc = ERROR_FAIL;
             break;

tools/libxl/libxl_dm.c +4 -0

@@ -257,6 +257,8 @@ static char ** libxl__build_device_model
         for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++)
             flexarray_append(dm_args, b_info->extra_hvm[i]);
         break;
+    default:
+        abort();
     }
     flexarray_append(dm_args, NULL);
     return (char **) flexarray_contents(dm_args);

@@ -505,6 +507,8 @@ static char ** libxl__build_device_model
         for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++)
             flexarray_append(dm_args, b_info->extra_hvm[i]);
         break;
+    default:
+        abort();
     }
 
     ram_size = libxl__sizekb_to_mb(b_info->max_memkb - b_info->video_memkb);

tools/libxl/libxl_dom.c +2 -2

@@ -33,9 +33,9 @@ libxl_domain_type libxl__domain_type(lib
 
     ret = xc_domain_getinfolist(ctx->xch, domid, 1, &info);
     if (ret != 1)
-        return -1;
+        return LIBXL_DOMAIN_TYPE_INVALID;
     if (info.domain != domid)
-        return -1;
+        return LIBXL_DOMAIN_TYPE_INVALID;
     if (info.flags & XEN_DOMINF_hvm_guest)
         return LIBXL_DOMAIN_TYPE_HVM;
     else

tools/libxl/libxl_types.idl +2 -0

@@ -28,6 +28,7 @@ MemKB = UInt(64, init_val = "LIBXL_MEMKB
 #
 
 libxl_domain_type = Enumeration("domain_type", [
+    (-1, "INVALID"),
     (1, "HVM"),
     (2, "PV"),
     ])

@@ -310,6 +311,7 @@ libxl_domain_build_info = Struct("domain
                                       # Use host's E820 for PCI passthrough.
                                       ("e820_host", libxl_defbool),
                                       ])),
+                 ("invalid", Struct(None, [])),
                  ], keyvar_init_val = "-1")),
     ], dir=DIR_IN
 )

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Ian Jackson

2012-06-06 10:43 UTC #2 lore raw

Re: [PATCH 1 of 1 v5] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID

Dario Faggioli writes ("[PATCH 1 of 1 v5] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID"):

Quoted 2 lines

To avoid recent gcc complaining about:
libxl.c: In function ‘libxl_primary_console_exec’:

Can you please avoid these non-ascii quotes in commit messages ? It makes my life unnecessarily difficult when applying patches. Any of `...', '...' and "..." are fine.

Ian.

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Dario Faggioli

author

2012-06-06 11:11 UTC #3 lore raw

Re: [PATCH 1 of 1 v5] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID

Quoted 8 lines · Ian Jackson

Dario Faggioli writes ("[PATCH 1 of 1 v5] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID"):
Quoted 2 lines
To avoid recent gcc complaining about:
libxl.c: In function ‘libxl_primary_console_exec’:
Can you please avoid these non-ascii quotes in commit messages ? It makes my life unnecessarily difficult when applying patches. Any of `...', '...' and "..." are fine.

Sure, sorry... Cut'n'pasting from gnome-terminal into vim has apparently produced monsters... :-/

Thanks and Regards,
Dario

Signature · 4 lines

<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Ian Jackson

2012-06-06 10:47 UTC #4 lore raw

Re: [PATCH 1 of 1 v5] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID

In reply to Dario Faggioli

Dario Faggioli writes ("[PATCH 1 of 1 v5] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID"):

Quoted 3 lines

To avoid recent gcc complaining about:
libxl.c: In function ‘libxl_primary_console_exec’:
libxl.c:1233:9: error: case value ‘4294967295’ not in enumerated type ‘libxl_domain_type’ [-Werror=switch]

...

tools/libxl/libxl.c int libxl_domain_remus_start(libxl_ctx * · line 663

Quoted 5 lines

tools/libxl/libxl.c +5 -0

+    if (type == LIBXL_DOMAIN_TYPE_INVALID) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                   "invalid domain type for domain %d", domid);
+        rc = ERROR_INVAL;
+        goto remus_fail;

This is not an expected error condition, is it ? ERROR_INVAL is for libxl being passed impromper parameters. So I think this should be ERROR_FAIL.

tools/libxl/libxl.c int libxl_domain_suspend(libxl_ctx *ctx, · line 707

Quoted 10 lines

tools/libxl/libxl.c +6 -0

@@ -692,11 +699,20 @@ int libxl_domain_suspend(libxl_ctx *ctx,
     int debug = info != NULL && info->flags & XL_SUSPEND_DEBUG;
     int rc = 0;
 
+    if (type == LIBXL_DOMAIN_TYPE_INVALID) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                   "invalid domain type for domain %d", domid);
+        rc = ERROR_INVAL;
+        goto suspend_fail;
+    }

Is it possible for you to leave this part alone ? It's moved about a lot in my suspend/resume series.

I will fix it up at the end of my series.

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Ian Campbell

2012-06-06 10:56 UTC #5 lore raw

Re: [PATCH 1 of 1 v5] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID

Quoted 11 lines · Ian Jackson

Dario Faggioli writes ("[PATCH 1 of 1 v5] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID"):
Quoted 3 lines
To avoid recent gcc complaining about:
libxl.c: In function ‘libxl_primary_console_exec’:
libxl.c:1233:9: error: case value ‘4294967295’ not in enumerated type ‘libxl_domain_type’ [-Werror=switch]
...
tools/libxl/libxl.c int libxl_domain_remus_start(libxl_ctx * · line 663
Quoted 5 lines
tools/libxl/libxl.c +5 -0
+    if (type == LIBXL_DOMAIN_TYPE_INVALID) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                   "invalid domain type for domain %d", domid);
+        rc = ERROR_INVAL;
+        goto remus_fail;

Quoted 3 lines

This is not an expected error condition, is it ? ERROR_INVAL is for libxl being passed impromper parameters. So I think this should be ERROR_FAIL.

Also perhaps the logging should be done in libxl__domain_type? Unless there are too many legitimate occasions where INVALID might come up?

Quoted 15 lines

tools/libxl/libxl.c int libxl_domain_suspend(libxl_ctx *ctx, · line 707

Quoted 11 lines

tools/libxl/libxl.c +6 -0

@@ -692,11 +699,20 @@ int libxl_domain_suspend(libxl_ctx *ctx,
     int debug = info != NULL && info->flags & XL_SUSPEND_DEBUG;
     int rc = 0;
 
+    if (type == LIBXL_DOMAIN_TYPE_INVALID) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                   "invalid domain type for domain %d", domid);
+        rc = ERROR_INVAL;
+        goto suspend_fail;
+    }

Is it possible for you to leave this part alone ? It's moved about a lot in my suspend/resume series.

Quoted 4 lines

I will fix it up at the end of my series.
Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Dario Faggioli

author

2012-06-06 11:15 UTC #6 lore raw

Re: [PATCH 1 of 1 v5] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID

Quoted 12 lines · Ian Campbell

Quoted 11 lines · Ian Jackson
Dario Faggioli writes ("[PATCH 1 of 1 v5] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID"):
Quoted 3 lines
To avoid recent gcc complaining about:
libxl.c: In function ‘libxl_primary_console_exec’:
libxl.c:1233:9: error: case value ‘4294967295’ not in enumerated type ‘libxl_domain_type’ [-Werror=switch]
...
Quoted 5 lines
tools/libxl/libxl.c +5 -0
+    if (type == LIBXL_DOMAIN_TYPE_INVALID) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                   "invalid domain type for domain %d", domid);
+        rc = ERROR_INVAL;
+        goto remus_fail;

Quoted 6 lines

Quoted 3 lines
This is not an expected error condition, is it ? ERROR_INVAL is for libxl being passed impromper parameters. So I think this should be ERROR_FAIL.
Also perhaps the logging should be done in libxl__domain_type?

That makes sense to me...

Quoted 3 lines

Unless
there are too many legitimate occasions where INVALID might come up?

I might be wrong but I don't think so. There might be some more printing than with this patch, but mostly before some 'default: abort();' situation so...

Anyway, I'll do it and try to see how it behaves.

Thanks and Regards,
Dario

Signature · 4 lines

<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Dario Faggioli

author

2012-06-06 11:10 UTC #7 lore raw

Re: [PATCH 1 of 1 v5] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID

In reply to Ian Jackson

Quoted 11 lines · Ian Jackson

Dario Faggioli writes ("[PATCH 1 of 1 v5] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID"):
Quoted 3 lines
To avoid recent gcc complaining about:
libxl.c: In function ‘libxl_primary_console_exec’:
libxl.c:1233:9: error: case value ‘4294967295’ not in enumerated type ‘libxl_domain_type’ [-Werror=switch]
...
tools/libxl/libxl.c int libxl_domain_remus_start(libxl_ctx * · line 663
Quoted 5 lines
tools/libxl/libxl.c +5 -0
+    if (type == LIBXL_DOMAIN_TYPE_INVALID) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                   "invalid domain type for domain %d", domid);
+        rc = ERROR_INVAL;
+        goto remus_fail;

Quoted 4 lines

This is not an expected error condition, is it ? ERROR_INVAL is for libxl being passed impromper parameters. So I think this should be ERROR_FAIL.

Sounds reasonable, will fix.

Quoted 14 lines

tools/libxl/libxl.c int libxl_domain_suspend(libxl_ctx *ctx, · line 707

Quoted 10 lines

tools/libxl/libxl.c +6 -0

@@ -692,11 +699,20 @@ int libxl_domain_suspend(libxl_ctx *ctx,
     int debug = info != NULL && info->flags & XL_SUSPEND_DEBUG;
     int rc = 0;
 
+    if (type == LIBXL_DOMAIN_TYPE_INVALID) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                   "invalid domain type for domain %d", domid);
+        rc = ERROR_INVAL;
+        goto suspend_fail;
+    }

Is it possible for you to leave this part alone ? It's moved about a lot in my suspend/resume series.

Quoted 2 lines

I will fix it up at the end of my series.

Ok.

Thanks and Regards,
Dario

Signature · 4 lines

<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel