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