[Spice-devel] [PATCH spice-server v2] stat-file: Avoid compiler warning

[Frediano Ziglio]

Some gcc version reports this error:

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.

This patch rewrite the loop in order to make more compilers
understand that node variable is always initialized.

There are two checks apparently removed in the patch:
- check for stat_file->stat not being NULL. This was worthless as the
  field was already used in the function. Also this field is never
  NULL (unless memory corruption happened);
- stat_file->stat->num_of_nodes >= stat_file->max_nodes. It's implicit
  in the loop. If num_of_nodes >= max_nodes means that there are no
  free nodes so all nodes should have SPICE_STAT_NODE_FLAG_ENABLED set,
  loop will exit and function will return INVALID_STAT_REF.

Reported-by: Christophe Fergeau <cfergeau at redhat.com>
Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
---
 server/stat-file.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

Changes since v1:
- improved commit message.

diff --git a/server/stat-file.c b/server/stat-file.c
index 3fe3890..96c3bc1 100644
--- a/server/stat-file.c
+++ b/server/stat-file.c
@@ -162,25 +162,23 @@ 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;
+        if (!!(node->flags & SPICE_STAT_NODE_FLAG_ENABLED)) {
+            continue;
         }
+        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 *
-- 
2.9.3