[libvirt] [PATCH v2 04/14] esx: handle missing switch enum cases

Daniel P. Berrangé posted 14 patches 7 years, 2 months ago
[libvirt] [PATCH v2 04/14] esx: handle missing switch enum cases
Posted by Daniel P. Berrangé 7 years, 2 months ago
Ensure all enum cases are listed in switch statements, or explicitly
cast away enum type where we don't want to list all cases.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/esx/esx_driver.c   |  1 +
 src/esx/esx_vi.c       | 11 +++++++----
 src/esx/esx_vi_types.c |  9 +++++----
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index f575362059..c5f04efa81 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -3603,6 +3603,7 @@ esxDomainGetSchedulerParametersFlags(virDomainPtr domain,
                 params[i].value.i = -3;
                 break;
 
+              case esxVI_SharesLevel_Undefined:
               default:
                 virReportError(VIR_ERR_INTERNAL_ERROR,
                                _("Shares level has unknown value %d"),
diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
index edf52ff828..103f726069 100644
--- a/src/esx/esx_vi.c
+++ b/src/esx/esx_vi.c
@@ -483,7 +483,7 @@ esxVI_SharedCURL_Lock(CURL *handle ATTRIBUTE_UNUSED, curl_lock_data data,
     size_t i;
     esxVI_SharedCURL *shared = userptr;
 
-    switch (data) {
+    switch ((int)data) {
       case CURL_LOCK_DATA_SHARE:
         i = 0;
         break;
@@ -511,7 +511,7 @@ esxVI_SharedCURL_Unlock(CURL *handle ATTRIBUTE_UNUSED, curl_lock_data data,
     size_t i;
     esxVI_SharedCURL *shared = userptr;
 
-    switch (data) {
+    switch ((int)data) {
       case CURL_LOCK_DATA_SHARE:
         i = 0;
         break;
@@ -1563,6 +1563,7 @@ esxVI_Context_Execute(esxVI_Context *ctx, const char *methodName,
 
                 break;
 
+              case esxVI_Occurrence_Undefined:
               default:
                 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                                _("Invalid argument (occurrence)"));
@@ -2280,9 +2281,11 @@ esxVI_LookupObjectContentByType(esxVI_Context *ctx,
                            type, root->type);
             break;
 
+          case esxVI_Occurrence_None:
+          case esxVI_Occurrence_Undefined:
           default:
-            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                           _("Invalid occurrence value"));
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Invalid occurrence value %d"), occurrence);
             break;
         }
 
diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c
index be35af861c..74183f8307 100644
--- a/src/esx/esx_vi_types.c
+++ b/src/esx/esx_vi_types.c
@@ -544,7 +544,7 @@ VIR_LOG_INIT("esx.esx_vi_types");
 
 #define ESX_VI__TEMPLATE__DISPATCH(_actual_type, _actual_type_name, __type, \
                                    _dispatch,  _error_return) \
-    switch (_actual_type) { \
+    switch ((int)_actual_type) { \
       _dispatch \
  \
       case esxVI_Type_##__type: \
@@ -690,7 +690,7 @@ VIR_LOG_INIT("esx.esx_vi_types");
           return -1; \
       } \
  \
-      switch (type) { \
+      switch ((int)type) { \
         _dispatch \
  \
         case esxVI_Type_##__type: \
@@ -967,7 +967,7 @@ esxVI_AnyType_DeepCopy(esxVI_AnyType **dest, esxVI_AnyType *src)
         goto failure;
     }
 
-    switch (src->type) {
+    switch ((int)src->type) {
       case esxVI_Type_Boolean:
         (*dest)->boolean = src->boolean;
         break;
@@ -1071,7 +1071,7 @@ esxVI_AnyType_Deserialize(xmlNodePtr node, esxVI_AnyType **anyType)
             (*anyType)->_name = number; \
         } while (0)
 
-    switch ((*anyType)->type) {
+    switch ((int)(*anyType)->type) {
       case esxVI_Type_Boolean:
         if (STREQ((*anyType)->value, "true")) {
             (*anyType)->boolean = esxVI_Boolean_True;
@@ -1876,6 +1876,7 @@ esxVI_VirtualMachinePowerState_ConvertToLibvirt
       case esxVI_VirtualMachinePowerState_Suspended:
         return VIR_DOMAIN_PAUSED;
 
+    case esxVI_VirtualMachinePowerState_Undefined:
       default:
         return VIR_DOMAIN_NOSTATE;
     }
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 04/14] esx: handle missing switch enum cases
Posted by John Ferlan 7 years, 2 months ago

On 02/20/2018 12:08 PM, Daniel P. Berrangé wrote:
> Ensure all enum cases are listed in switch statements, or explicitly
> cast away enum type where we don't want to list all cases.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  src/esx/esx_driver.c   |  1 +
>  src/esx/esx_vi.c       | 11 +++++++----
>  src/esx/esx_vi_types.c |  9 +++++----
>  3 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
> index f575362059..c5f04efa81 100644
> --- a/src/esx/esx_driver.c
> +++ b/src/esx/esx_driver.c
> @@ -3603,6 +3603,7 @@ esxDomainGetSchedulerParametersFlags(virDomainPtr domain,
>                  params[i].value.i = -3;
>                  break;
>  
> +              case esxVI_SharesLevel_Undefined:
>                default:
>                  virReportError(VIR_ERR_INTERNAL_ERROR,
>                                 _("Shares level has unknown value %d"),

Shouldn't this be:

   virReportEnumRangeError(esxVI_SharesLevel, sharesInfo->level);

> diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
> index edf52ff828..103f726069 100644
> --- a/src/esx/esx_vi.c
> +++ b/src/esx/esx_vi.c
> @@ -483,7 +483,7 @@ esxVI_SharedCURL_Lock(CURL *handle ATTRIBUTE_UNUSED, curl_lock_data data,
>      size_t i;
>      esxVI_SharedCURL *shared = userptr;
>  
> -    switch (data) {
> +    switch ((int)data) {
>        case CURL_LOCK_DATA_SHARE:
>          i = 0;
>          break;
> @@ -511,7 +511,7 @@ esxVI_SharedCURL_Unlock(CURL *handle ATTRIBUTE_UNUSED, curl_lock_data data,
>      size_t i;
>      esxVI_SharedCURL *shared = userptr;
>  
> -    switch (data) {
> +    switch ((int)data) {
>        case CURL_LOCK_DATA_SHARE:
>          i = 0;
>          break;
> @@ -1563,6 +1563,7 @@ esxVI_Context_Execute(esxVI_Context *ctx, const char *methodName,
>  
>                  break;
>  
> +              case esxVI_Occurrence_Undefined:
>                default:
>                  virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                                 _("Invalid argument (occurrence)"));

and:

    virReportEnumRangeError(esxVI_Occurrence, occurrence);

> @@ -2280,9 +2281,11 @@ esxVI_LookupObjectContentByType(esxVI_Context *ctx,
>                             type, root->type);
>              break;
>  
> +          case esxVI_Occurrence_None:
> +          case esxVI_Occurrence_Undefined:
>            default:
> -            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                           _("Invalid occurrence value"));
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Invalid occurrence value %d"), occurrence);

and:

    virReportEnumRangeError(esxVI_Occurrence, occurrence);

>              break;
>          }
>  
> diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c
> index be35af861c..74183f8307 100644
> --- a/src/esx/esx_vi_types.c
> +++ b/	
> @@ -544,7 +544,7 @@ VIR_LOG_INIT("esx.esx_vi_types");
>  
>  #define ESX_VI__TEMPLATE__DISPATCH(_actual_type, _actual_type_name, __type, \
>                                     _dispatch,  _error_return) \
> -    switch (_actual_type) { \
> +    switch ((int)_actual_type) { \
>        _dispatch \
>   \
>        case esxVI_Type_##__type: \
> @@ -690,7 +690,7 @@ VIR_LOG_INIT("esx.esx_vi_types");
>            return -1; \
>        } \
>   \
> -      switch (type) { \
> +      switch ((int)type) { \
>          _dispatch \
>   \
>          case esxVI_Type_##__type: \
> @@ -967,7 +967,7 @@ esxVI_AnyType_DeepCopy(esxVI_AnyType **dest, esxVI_AnyType *src)
>          goto failure;
>      }
>  
> -    switch (src->type) {
> +    switch ((int)src->type) {
>        case esxVI_Type_Boolean:
>          (*dest)->boolean = src->boolean;
>          break;
> @@ -1071,7 +1071,7 @@ esxVI_AnyType_Deserialize(xmlNodePtr node, esxVI_AnyType **anyType)
>              (*anyType)->_name = number; \
>          } while (0)
>  
> -    switch ((*anyType)->type) {
> +    switch ((int)(*anyType)->type) {
>        case esxVI_Type_Boolean:
>          if (STREQ((*anyType)->value, "true")) {
>              (*anyType)->boolean = esxVI_Boolean_True;
> @@ -1876,6 +1876,7 @@ esxVI_VirtualMachinePowerState_ConvertToLibvirt
>        case esxVI_VirtualMachinePowerState_Suspended:
>          return VIR_DOMAIN_PAUSED;
>  
> +    case esxVI_VirtualMachinePowerState_Undefined:

Fix indent - off by 2.

Reviewed-by: John Ferlan <jferlan@redhat.com>

John

>        default:
>          return VIR_DOMAIN_NOSTATE;
>      }
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 04/14] esx: handle missing switch enum cases
Posted by Daniel P. Berrangé 7 years, 2 months ago
On Tue, Feb 20, 2018 at 02:03:19PM -0500, John Ferlan wrote:
> 
> 
> On 02/20/2018 12:08 PM, Daniel P. Berrangé wrote:
> > Ensure all enum cases are listed in switch statements, or explicitly
> > cast away enum type where we don't want to list all cases.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  src/esx/esx_driver.c   |  1 +
> >  src/esx/esx_vi.c       | 11 +++++++----
> >  src/esx/esx_vi_types.c |  9 +++++----
> >  3 files changed, 13 insertions(+), 8 deletions(-)
> > 
> > diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
> > index f575362059..c5f04efa81 100644
> > --- a/src/esx/esx_driver.c
> > +++ b/src/esx/esx_driver.c
> > @@ -3603,6 +3603,7 @@ esxDomainGetSchedulerParametersFlags(virDomainPtr domain,
> >                  params[i].value.i = -3;
> >                  break;
> >  
> > +              case esxVI_SharesLevel_Undefined:
> >                default:
> >                  virReportError(VIR_ERR_INTERNAL_ERROR,
> >                                 _("Shares level has unknown value %d"),
> 
> Shouldn't this be:
> 
>    virReportEnumRangeError(esxVI_SharesLevel, sharesInfo->level);

Yeah, I had only changed messages I added in v1, but thse can be touched
up too.

> 
> > diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
> > index edf52ff828..103f726069 100644
> > --- a/src/esx/esx_vi.c
> > +++ b/src/esx/esx_vi.c
> > @@ -483,7 +483,7 @@ esxVI_SharedCURL_Lock(CURL *handle ATTRIBUTE_UNUSED, curl_lock_data data,
> >      size_t i;
> >      esxVI_SharedCURL *shared = userptr;
> >  
> > -    switch (data) {
> > +    switch ((int)data) {
> >        case CURL_LOCK_DATA_SHARE:
> >          i = 0;
> >          break;
> > @@ -511,7 +511,7 @@ esxVI_SharedCURL_Unlock(CURL *handle ATTRIBUTE_UNUSED, curl_lock_data data,
> >      size_t i;
> >      esxVI_SharedCURL *shared = userptr;
> >  
> > -    switch (data) {
> > +    switch ((int)data) {
> >        case CURL_LOCK_DATA_SHARE:
> >          i = 0;
> >          break;
> > @@ -1563,6 +1563,7 @@ esxVI_Context_Execute(esxVI_Context *ctx, const char *methodName,
> >  
> >                  break;
> >  
> > +              case esxVI_Occurrence_Undefined:
> >                default:
> >                  virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> >                                 _("Invalid argument (occurrence)"));
> 
> and:
> 
>     virReportEnumRangeError(esxVI_Occurrence, occurrence);
> 
> > @@ -2280,9 +2281,11 @@ esxVI_LookupObjectContentByType(esxVI_Context *ctx,
> >                             type, root->type);
> >              break;
> >  
> > +          case esxVI_Occurrence_None:
> > +          case esxVI_Occurrence_Undefined:
> >            default:
> > -            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > -                           _("Invalid occurrence value"));
> > +            virReportError(VIR_ERR_INTERNAL_ERROR,
> > +                           _("Invalid occurrence value %d"), occurrence);
> 
> and:
> 
>     virReportEnumRangeError(esxVI_Occurrence, occurrence);
> 
> >              break;
> >          }
> >  
> > diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c
> > index be35af861c..74183f8307 100644
> > --- a/src/esx/esx_vi_types.c
> > +++ b/	
> > @@ -544,7 +544,7 @@ VIR_LOG_INIT("esx.esx_vi_types");
> >  
> >  #define ESX_VI__TEMPLATE__DISPATCH(_actual_type, _actual_type_name, __type, \
> >                                     _dispatch,  _error_return) \
> > -    switch (_actual_type) { \
> > +    switch ((int)_actual_type) { \
> >        _dispatch \
> >   \
> >        case esxVI_Type_##__type: \
> > @@ -690,7 +690,7 @@ VIR_LOG_INIT("esx.esx_vi_types");
> >            return -1; \
> >        } \
> >   \
> > -      switch (type) { \
> > +      switch ((int)type) { \
> >          _dispatch \
> >   \
> >          case esxVI_Type_##__type: \
> > @@ -967,7 +967,7 @@ esxVI_AnyType_DeepCopy(esxVI_AnyType **dest, esxVI_AnyType *src)
> >          goto failure;
> >      }
> >  
> > -    switch (src->type) {
> > +    switch ((int)src->type) {
> >        case esxVI_Type_Boolean:
> >          (*dest)->boolean = src->boolean;
> >          break;
> > @@ -1071,7 +1071,7 @@ esxVI_AnyType_Deserialize(xmlNodePtr node, esxVI_AnyType **anyType)
> >              (*anyType)->_name = number; \
> >          } while (0)
> >  
> > -    switch ((*anyType)->type) {
> > +    switch ((int)(*anyType)->type) {
> >        case esxVI_Type_Boolean:
> >          if (STREQ((*anyType)->value, "true")) {
> >              (*anyType)->boolean = esxVI_Boolean_True;
> > @@ -1876,6 +1876,7 @@ esxVI_VirtualMachinePowerState_ConvertToLibvirt
> >        case esxVI_VirtualMachinePowerState_Suspended:
> >          return VIR_DOMAIN_PAUSED;
> >  
> > +    case esxVI_VirtualMachinePowerState_Undefined:
> 
> Fix indent - off by 2.
> 
> Reviewed-by: John Ferlan <jferlan@redhat.com>
> 
> John
> 
> >        default:
> >          return VIR_DOMAIN_NOSTATE;
> >      }
> > 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list