Ensure that all switch statements in this module use the proper type in
switch() statements to ensure complier protections.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/util/virtypedparam.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c
index 0cca16053d..974ec51a79 100644
--- a/src/util/virtypedparam.c
+++ b/src/util/virtypedparam.c
@@ -170,7 +170,7 @@ virTypedParameterToString(virTypedParameterPtr param)
 {
     char *value = NULL;

-    switch (param->type) {
+    switch ((virTypedParameterType) param->type) {
     case VIR_TYPED_PARAM_INT:
         value = g_strdup_printf("%d", param->value.i);
         break;
@@ -192,6 +192,7 @@ virTypedParameterToString(virTypedParameterPtr param)
     case VIR_TYPED_PARAM_STRING:
         value = g_strdup(param->value.s);
         break;
+    case VIR_TYPED_PARAM_LAST:
     default:
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("unexpected type %1$d for field %2$s"),
@@ -204,7 +205,7 @@ virTypedParameterToString(virTypedParameterPtr param)

 static int
 virTypedParameterAssignValueVArgs(virTypedParameterPtr param,
-                                  int type,
+                                  virTypedParameterType type,
                                   va_list ap,
                                   bool copystr)
 {
@@ -238,6 +239,7 @@ virTypedParameterAssignValueVArgs(virTypedParameterPtr param,
         if (!param->value.s)
             param->value.s = g_strdup("");
         break;
+    case VIR_TYPED_PARAM_LAST:
     default:
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("unexpected type %1$d for field %2$s"), type,
@@ -559,7 +561,7 @@ virTypedParamsDeserialize(struct _virTypedParameterRemote *remote_params,
         }

         param->type = remote_param->value.type;
-        switch (param->type) {
+        switch ((virTypedParameterType) param->type) {
         case VIR_TYPED_PARAM_INT:
             param->value.i =
                 remote_param->value.remote_typed_param_value.i;
@@ -587,6 +589,7 @@ virTypedParamsDeserialize(struct _virTypedParameterRemote *remote_params,
         case VIR_TYPED_PARAM_STRING:
             param->value.s = g_strdup(remote_param->value.remote_typed_param_value.s);
             break;
+        case VIR_TYPED_PARAM_LAST:
         default:
             virReportError(VIR_ERR_RPC, _("unknown parameter type: %1$d"),
                            param->type);
@@ -670,7 +673,7 @@ virTypedParamsSerialize(virTypedParameterPtr params,
          * depending on the calling side, i.e. server or client */
         val->field = g_strdup(param->field);
         val->value.type = param->type;
-        switch (param->type) {
+        switch ((virTypedParameterType) param->type) {
         case VIR_TYPED_PARAM_INT:
             val->value.remote_typed_param_value.i = param->value.i;
             break;
@@ -692,6 +695,7 @@ virTypedParamsSerialize(virTypedParameterPtr params,
         case VIR_TYPED_PARAM_STRING:
             val->value.remote_typed_param_value.s = g_strdup(param->value.s);
             break;
+        case VIR_TYPED_PARAM_LAST:
         default:
             virReportError(VIR_ERR_RPC, _("unknown parameter type: %1$d"),
                            param->type);
-- 
2.39.2

On Wed, Apr 19, 2023 at 02:04:19PM +0200, Peter Krempa wrote:
>Ensure that all switch statements in this module use the proper type in
>switch() statements to ensure complier protections.
>
>Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>---
> src/util/virtypedparam.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
>diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c
>index 0cca16053d..974ec51a79 100644
>--- a/src/util/virtypedparam.c
>+++ b/src/util/virtypedparam.c
>@@ -170,7 +170,7 @@ virTypedParameterToString(virTypedParameterPtr param)
> {
>     char *value = NULL;
>
>-    switch (param->type) {
>+    switch ((virTypedParameterType) param->type) {
>     case VIR_TYPED_PARAM_INT:
>         value = g_strdup_printf("%d", param->value.i);
>         break;
>@@ -192,6 +192,7 @@ virTypedParameterToString(virTypedParameterPtr param)
>     case VIR_TYPED_PARAM_STRING:
>         value = g_strdup(param->value.s);
>         break;
>+    case VIR_TYPED_PARAM_LAST:
>     default:

To ensure compiler protection in switch() statements you should also
remove the default case from the switch statements.

With that changed:

Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

On Wed, Apr 19, 2023 at 15:02:34 +0200, Martin Kletzander wrote:
> On Wed, Apr 19, 2023 at 02:04:19PM +0200, Peter Krempa wrote:
> > Ensure that all switch statements in this module use the proper type in
> > switch() statements to ensure complier protections.
> > 
> > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > ---
> > src/util/virtypedparam.c | 12 ++++++++----
> > 1 file changed, 8 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c
> > index 0cca16053d..974ec51a79 100644
> > --- a/src/util/virtypedparam.c
> > +++ b/src/util/virtypedparam.c
> > @@ -170,7 +170,7 @@ virTypedParameterToString(virTypedParameterPtr param)
> > {
> >     char *value = NULL;
> > 
> > -    switch (param->type) {
> > +    switch ((virTypedParameterType) param->type) {
> >     case VIR_TYPED_PARAM_INT:
> >         value = g_strdup_printf("%d", param->value.i);
> >         break;
> > @@ -192,6 +192,7 @@ virTypedParameterToString(virTypedParameterPtr param)
> >     case VIR_TYPED_PARAM_STRING:
> >         value = g_strdup(param->value.s);
> >         break;
> > +    case VIR_TYPED_PARAM_LAST:
> >     default:
> 
> To ensure compiler protection in switch() statements you should also
> remove the default case from the switch statements.

It actually works properly even with the 'default' case being present.
Additionally in case a caller passes in something which is not actually
a proper enum value (e.g. by typecasting) the 'default' case will be
applied.

In many new places we already do it like this.

> 
> With that changed:
> 
> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

On Wed, Apr 19, 2023 at 03:08:03PM +0200, Peter Krempa wrote:
>On Wed, Apr 19, 2023 at 15:02:34 +0200, Martin Kletzander wrote:
>> On Wed, Apr 19, 2023 at 02:04:19PM +0200, Peter Krempa wrote:
>> > Ensure that all switch statements in this module use the proper type in
>> > switch() statements to ensure complier protections.
>> >
>> > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>> > ---
>> > src/util/virtypedparam.c | 12 ++++++++----
>> > 1 file changed, 8 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c
>> > index 0cca16053d..974ec51a79 100644
>> > --- a/src/util/virtypedparam.c
>> > +++ b/src/util/virtypedparam.c
>> > @@ -170,7 +170,7 @@ virTypedParameterToString(virTypedParameterPtr param)
>> > {
>> >     char *value = NULL;
>> >
>> > -    switch (param->type) {
>> > +    switch ((virTypedParameterType) param->type) {
>> >     case VIR_TYPED_PARAM_INT:
>> >         value = g_strdup_printf("%d", param->value.i);
>> >         break;
>> > @@ -192,6 +192,7 @@ virTypedParameterToString(virTypedParameterPtr param)
>> >     case VIR_TYPED_PARAM_STRING:
>> >         value = g_strdup(param->value.s);
>> >         break;
>> > +    case VIR_TYPED_PARAM_LAST:
>> >     default:
>>
>> To ensure compiler protection in switch() statements you should also
>> remove the default case from the switch statements.
>
>It actually works properly even with the 'default' case being present.
>Additionally in case a caller passes in something which is not actually
>a proper enum value (e.g. by typecasting) the 'default' case will be
>applied.
>

Oh, good to know, TIL then.  In the meantime I replied as well =)

>In many new places we already do it like this.
>
>>
>> With that changed:
>>
>> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
>
>

On Wed, Apr 19, 2023 at 03:02:34PM +0200, Martin Kletzander wrote:
>On Wed, Apr 19, 2023 at 02:04:19PM +0200, Peter Krempa wrote:
>>Ensure that all switch statements in this module use the proper type in
>>switch() statements to ensure complier protections.
>>
>>Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>>---
>> src/util/virtypedparam.c | 12 ++++++++----
>> 1 file changed, 8 insertions(+), 4 deletions(-)
>>
>>diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c
>>index 0cca16053d..974ec51a79 100644
>>--- a/src/util/virtypedparam.c
>>+++ b/src/util/virtypedparam.c
>>@@ -170,7 +170,7 @@ virTypedParameterToString(virTypedParameterPtr param)
>> {
>>     char *value = NULL;
>>
>>-    switch (param->type) {
>>+    switch ((virTypedParameterType) param->type) {
>>     case VIR_TYPED_PARAM_INT:
>>         value = g_strdup_printf("%d", param->value.i);
>>         break;
>>@@ -192,6 +192,7 @@ virTypedParameterToString(virTypedParameterPtr param)
>>     case VIR_TYPED_PARAM_STRING:
>>         value = g_strdup(param->value.s);
>>         break;
>>+    case VIR_TYPED_PARAM_LAST:
>>     default:
>
>To ensure compiler protection in switch() statements you should also
>remove the default case from the switch statements.
>
>With that changed:
>
>Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

I missed that the type is an int which is not checked anywhere else, so
the default branch needs to stay there, unfortunately that will not
ensure compiler protection, except in that one case which you fix in a
follow-up.  So feel free to keep this one as is even though the commit
message is a bit misleading.