thr3ads.net - Xen devel - [PATCH 0 of 1 v5] Fix build failure with gcc''s -Werror=switch [Jun 2012]

If this information is useful, please help other people find it:
Share via:

Dario Faggioli

2012-Jun-04 17:23 UTC

[PATCH 0 of 1 v5] 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).

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

-- 
<<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

2012-Jun-04 17:23 UTC

head link

[PATCH 1 of 1 v5] 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 <dario.faggioli@citrix.com>
Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -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;
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -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);
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -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
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -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
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Ian Jackson

2012-Jun-06 10:43 UTC

head link

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"):> 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
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Ian Jackson

2012-Jun-06 10:47 UTC

head link

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"):> 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]
...> +    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.
> @@ -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
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Ian Campbell

2012-Jun-06 10:56 UTC

head link

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

On Wed, 2012-06-06 at 11:47 +0100, Ian Jackson wrote:> Dario Faggioli writes ("[PATCH 1 of 1 v5] 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]
> ...
> > +    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.
Also perhaps the logging should be done in libxl__domain_type? Unless
there are too many legitimate occasions where INVALID might come up?
> 
> > @@ -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
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Dario Faggioli

2012-Jun-06 11:10 UTC

head link

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

On Wed, 2012-06-06 at 11:47 +0100, Ian Jackson wrote:> Dario Faggioli writes ("[PATCH 1 of 1 v5] 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]
> ...
> > +    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.
> Sounds reasonable, will fix.
> > @@ -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.
> Ok.

Thanks and Regards,
Dario

-- 
<<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
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Dario Faggioli

2012-Jun-06 11:11 UTC

head link

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

On Wed, 2012-06-06 at 11:43 +0100, Ian Jackson wrote:> Dario Faggioli writes ("[PATCH 1 of 1 v5] libxl: introduce
LIBXL_DOMAIN_TYPE_INVALID"):
> > 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

-- 
<<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
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Dario Faggioli

2012-Jun-06 11:15 UTC

head link

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

On Wed, 2012-06-06 at 11:56 +0100, Ian Campbell wrote:> On Wed, 2012-06-06 at 11:47 +0100, Ian Jackson wrote:
> > Dario Faggioli writes ("[PATCH 1 of 1 v5] 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]
> > ...
> > > +    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.
> 
> Also perhaps the logging should be done in libxl__domain_type? 
>That makes sense to me...
> 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

-- 
<<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
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Xen devel - Jun 2012 - [PATCH 0 of 1 v5] Fix build failure with gcc's -Werror=switch

[PATCH 0 of 1 v5] Fix build failure with gcc''s -Werror=switch

[PATCH 1 of 1 v5] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID

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

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

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

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

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

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