[Spice-devel] [spice-server] stat: Silence gcc warning

[Christophe Fergeau]

On Mon, Jan 23, 2017 at 05:39:25AM -0500, Frediano Ziglio wrote:
> > 
> > On 01/20/2017 05:30 PM, Christophe Fergeau 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
> > 
> > Hi Christophe,
> > 
> > Looks good to me.
> > 
> 
> it's actually a false positive, from 

Yes, I can say it explicitly in the commit log, but by "silence", I
implicitly meant this warning should not be emitted by gcc in the first
place.

> 
>     for (ref = 0; ref <= stat_file->max_nodes; ref++) {
>         node = &stat_file->stat->nodes[ref];
> 
> node is always initialized as ref and max_nodes are unsigned so
> at the beginning ref == 0 and start_file->max_nodes >= 0 so
> ref <= 0 <= start_file->max_nodes and node is initialized.
> 
> > Some comments:
> > 1. It would be nice to (unlock and) return if node == NULL below.
> 
> as said node cannot be NULL and by the way if it was NULL node->value
> would crash so return is not reached.
> 
> > 2. (In the area) the check for  (stat_file->stat == NULL) should
> >     be before the while
> 
> agreed, I would put 
> 
>     if (stat_file->stat == NULL || strlen(name) >= sizeof(node->name)) {
>         return INVALID_STAT_REF;
>     }   

Dunno if we are allowed to check stat_file->stat value without holding
the lock? Should be fine as once it's created, we never seem to change
stat_file->stat, but who knows :)

Christophe
-------------- 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/20170123/274f32e4/attachment.sig>