[Spice-devel] [spice-server v3 1/2] stat: Silence gcc warning

[Christophe Fergeau]

On Tue, Jan 24, 2017 at 10:43:42AM -0500, Frediano Ziglio wrote:
> > 
> > Initializing 'node' to NULL silences this warning:
> > 
> > stat-file.c: In function 'stat_file_add_node':
> > stat-file.c:180:15: error: 'node' may be used uninitialized in this function
> > [-Werror=maybe-uninitialized]
> >      g_strlcpy(node->name, name, sizeof(node->name));
> >                ^~~~
> > cc1: all warnings being treated as errors
> > 
> > This warning is a false positive as this loop:
> >     for (ref = 0; ref <= stat_file->max_nodes; ref++) {
> >         node = &stat_file->stat->nodes[ref];
> >         ...
> >     }
> > will always iterate at least once.
> > ---
> >  server/stat-file.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/server/stat-file.c b/server/stat-file.c
> > index c23f4f5..de455b6 100644
> > --- a/server/stat-file.c
> > +++ b/server/stat-file.c
> > @@ -144,7 +144,7 @@ StatNodeRef
> >  stat_file_add_node(RedStatFile *stat_file, StatNodeRef parent, const char
> >  *name, int visible)
> >  {
> >      StatNodeRef ref;
> > -    SpiceStatNode *node;
> > +    SpiceStatNode *node = NULL;
> >  
> >      spice_assert(name && strlen(name) > 0);
> >      if (strlen(name) >= sizeof(node->name)) {
> 
> Something like that could work?

This works too, though I prefer the current flow
if (error)
  return ERROR;

/* no error case */

better than

if (success)
  return SUCCESS

/* finish handling error state */

But yeah, this patch is fine with me if you prefer to go this way.

Christophe

> 
> 
> diff --git a/server/stat-file.c b/server/stat-file.c
> index c23f4f5..8093319 100644
> --- a/server/stat-file.c
> +++ b/server/stat-file.c
> @@ -162,25 +162,22 @@ stat_file_add_node(RedStatFile *stat_file, StatNodeRef parent, const char *name,
>              return ref;
>          }
>      }
> -    if (stat_file->stat->num_of_nodes >= stat_file->max_nodes || stat_file->stat == NULL) {
> -        pthread_mutex_unlock(&stat_file->lock);
> -        return INVALID_STAT_REF;
> -    }
> -    stat_file->stat->generation++;
> -    stat_file->stat->num_of_nodes++;
>      for (ref = 0; ref <= stat_file->max_nodes; ref++) {
>          node = &stat_file->stat->nodes[ref];
>          if (!(node->flags & SPICE_STAT_NODE_FLAG_ENABLED)) {
> -            break;
> +            stat_file->stat->generation++;
> +            stat_file->stat->num_of_nodes++;
> +            node->value = 0;
> +            node->flags = SPICE_STAT_NODE_FLAG_ENABLED |
> +                          (visible ? SPICE_STAT_NODE_FLAG_VISIBLE : 0);
> +            g_strlcpy(node->name, name, sizeof(node->name));
> +            reds_insert_stat_node(stat_file, parent, ref);
> +            pthread_mutex_unlock(&stat_file->lock);
> +            return ref;
>          }
>      }
> -    spice_assert(!(node->flags & SPICE_STAT_NODE_FLAG_ENABLED));
> -    node->value = 0;
> -    node->flags = SPICE_STAT_NODE_FLAG_ENABLED | (visible ? SPICE_STAT_NODE_FLAG_VISIBLE : 0);
> -    g_strlcpy(node->name, name, sizeof(node->name));
> -    reds_insert_stat_node(stat_file, parent, ref);
>      pthread_mutex_unlock(&stat_file->lock);
> -    return ref;
> +    return INVALID_STAT_REF;
>  }
>  
>  uint64_t *
> 
> 
> it's a bit longer but surely any compiler won't complain about not initialized.
> 
> Frediano
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170125/e38486ae/attachment.sig>