> >
> > Coverity complains node->flags is accessed without locking stat_file->lock.
> > However the function stat_file_add_node is only called (from
> > stat_file_add_counter) when the lock is taken.
> >
>
> No, this is false. The function handle the lock.
> The problem happens because stat_file_add_counter add a flag without locking.
> However in this case this is not much of a problem as the this is the last
> change of
> the flag and maximum cause a temporary view of the node as not counter.
>
> > Add comment so it's clear.
> >
> > Signed-off-by: Uri Lublin <uril@xxxxxxxxxx>
> > ---
> > server/stat-file.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/server/stat-file.c b/server/stat-file.c
> > index 2797fd739..9aff8cd72 100644
> > --- a/server/stat-file.c
> > +++ b/server/stat-file.c
> > @@ -139,6 +139,7 @@ static void reds_insert_stat_node(RedStatFile
> > *stat_file,
> > StatNodeRef parent, St
> > }
> > }
> >
> > +/* Called with stat_file->lock locked */
> > StatNodeRef
> > stat_file_add_node(RedStatFile *stat_file, StatNodeRef parent, const char
> > *name, int visible)
> > {
>
This should work
diff --git a/server/stat-file.c b/server/stat-file.c
index 2797fd73..c52c266c 100644
--- a/server/stat-file.c
+++ b/server/stat-file.c
@@ -139,8 +139,9 @@ static void reds_insert_stat_node(RedStatFile *stat_file, StatNodeRef parent, St
}
}
-StatNodeRef
-stat_file_add_node(RedStatFile *stat_file, StatNodeRef parent, const char *name, int visible)
+static StatNodeRef
+stat_file_add_node_flags(RedStatFile *stat_file, StatNodeRef parent, const char *name,
+ uint32_t flags)
{
StatNodeRef ref;
SpiceStatNode *node;
@@ -169,8 +170,7 @@ stat_file_add_node(RedStatFile *stat_file, StatNodeRef parent, const char *name,
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);
+ node->flags = SPICE_STAT_NODE_FLAG_ENABLED | flags;
g_strlcpy(node->name, name, sizeof(node->name));
reds_insert_stat_node(stat_file, parent, ref);
pthread_mutex_unlock(&stat_file->lock);
@@ -180,17 +180,25 @@ stat_file_add_node(RedStatFile *stat_file, StatNodeRef parent, const char *name,
return INVALID_STAT_REF;
}
+StatNodeRef
+stat_file_add_node(RedStatFile *stat_file, StatNodeRef parent, const char *name, int visible)
+{
+ uint32_t flags = visible ? SPICE_STAT_NODE_FLAG_VISIBLE : 0;
+ return stat_file_add_node_flags(stat_file, parent, name, flags);
+}
+
uint64_t *
stat_file_add_counter(RedStatFile *stat_file, StatNodeRef parent, const char *name, int visible)
{
- StatNodeRef ref = stat_file_add_node(stat_file, parent, name, visible);
+ uint32_t flags = visible ? SPICE_STAT_NODE_FLAG_VISIBLE : 0;
+ flags |= SPICE_STAT_NODE_FLAG_VALUE;
+ StatNodeRef ref = stat_file_add_node_flags(stat_file, parent, name, flags);
SpiceStatNode *node;
if (ref == INVALID_STAT_REF) {
return NULL;
}
node = &stat_file->stat->nodes[ref];
- node->flags |= SPICE_STAT_NODE_FLAG_VALUE;
return &node->value;
}
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel