On 02/02/2017 02:36 PM, Frediano Ziglio wrote:
On 02/02/2017 11:52 AM, Frediano Ziglio wrote:
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 the NULL check is useless.
Reported-by: Christophe Fergeau <cfergeau@xxxxxxxxxx>
Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
---
server/stat-file.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/server/stat-file.c b/server/stat-file.c
index c23f4f5..05ad0ef 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;
- }
Hi Frediano,
Why did you remove this check ?
I think it is important.
Uri.
It's implicit in the loop.
If num_of_nodes >= max_nodes means that there are no free nodes
so all nodes should haveSPICE_STAT_NODE_FLAG_ENABLED set,
loop will exit and function will return INVALID_STAT_REF.
However I just realized that the test ref <= stat_file->max_nodes
it's a off-by-one, should be ref < stat_file->max_nodes !!
Frediano
Right, the off-by-one in the loop is protected by this condition.
This is why it's important.
But I prefer to change the loop condition, as you suggest.
What about stat_file->stat,
Is it guaranteed to not be NULL ?
Uri.
- 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 *
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel