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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel