[Spice-commits] server/stat-file.c

[Frediano Ziglio]

 server/stat-file.c |   26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

New commits:
commit 6be1c2f839832177a999d6b334932392ae2d77b0
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Tue Jan 24 17:42:47 2017 +0000

    stat-file: Avoid compiler warning
    
    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>
    Acked-by: Jonathon Jongsma <jjongsma at redhat.com>

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 *