Acked-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
Thanks for the expanded commit log.
On Fri, 2017-02-03 at 09:59 +0000, 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 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@xxxxxxxxxx>
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> ---
> 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 *
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel