>
> On Mon, 2017-03-06 at 18:22 +0000, Frediano Ziglio wrote:
> > The default memset(0) on the node does not init it correctly.
> yeah, nice catch, INVALID_STAT_REF = ~0
>
> > Do so to avoid other nodes/counters appended to unwanted nodes.
> >
> > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> > ---
> > server/red-channel.c | 2 ++
> > server/stat.h | 8 ++++++++
> > 2 files changed, 10 insertions(+)
> >
> > diff --git a/server/red-channel.c b/server/red-channel.c
> > index 8ae6ece..8206f4c 100644
> > --- a/server/red-channel.c
> > +++ b/server/red-channel.c
> > @@ -300,6 +300,8 @@ red_channel_init(RedChannel *self)
> > self->priv->client_cbs.connect =
> > red_channel_client_default_connect;
> > self->priv->client_cbs.disconnect =
> > red_channel_client_default_disconnect;
> > self->priv->client_cbs.migrate =
> > red_channel_client_default_migrate;
> > +
> > + stat_init_node_invalid(&self->priv->stat);
> > }
> >
> >
> > diff --git a/server/stat.h b/server/stat.h
> > index 5255efa..980fb07 100644
> > --- a/server/stat.h
> > +++ b/server/stat.h
> > @@ -70,6 +70,14 @@ stat_remove_counter(SpiceServer *reds,
> > RedStatCounter *counter)
> > #endif /* RED_STATISTICS */
> >
> > static inline void
> > +stat_init_node_invalid(RedStatNode *node)
> maybe 'invalidate' instead of 'invalid', or _set_invalid
>
> Also the argument can be marked as unused
Yes,
> > +{
> > +#ifdef RED_STATISTICS
> > + node->ref = INVALID_STAT_REF;
> > +#endif
> > +}
> > +
> > +static inline void
> > stat_inc_counter(RedStatCounter counter, uint64_t value)
> > {
> > #ifdef RED_STATISTICS
>
> What about using / improving stat_init_node or stat_remove_node
>
> Wouldn't be easier to change the INVALID_STAT_REF define ?
>
>
> Thanks,
> Pavel
>
>
INVALID_STAT_REF is currently part of the ABI for the file.
Unless we use a different invalid for stat-file API one way
is this (less hacky than I though):
diff --git a/server/reds.c b/server/reds.c
index fc720a3..e910404 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -355,22 +355,23 @@ static void reds_link_free(RedLinkInfo *link)
void stat_init_node(RedStatNode *node, SpiceServer *reds, const RedStatNode *parent,
const char *name, int visible)
{
- StatNodeRef parent_ref = parent ? parent->ref : INVALID_STAT_REF;
- node->ref = stat_file_add_node(reds->stat_file, parent_ref, name, visible);
+ StatNodeRef parent_ref = (parent ? parent->masked_ref : 0) ^ INVALID_STAT_REF;
+ node->masked_ref =
+ stat_file_add_node(reds->stat_file, parent_ref, name, visible) ^ INVALID_STAT_REF;
}
void stat_remove_node(SpiceServer *reds, RedStatNode *node)
{
- if (node->ref != INVALID_STAT_REF) {
- stat_file_remove_node(reds->stat_file, node->ref);
- node->ref = INVALID_STAT_REF;
+ if (node->masked_ref) {
+ stat_file_remove_node(reds->stat_file, node->masked_ref ^ INVALID_STAT_REF);
+ node->masked_ref = 0;
}
}
void stat_init_counter(RedStatCounter *counter, SpiceServer *reds,
const RedStatNode *parent, const char *name, int visible)
{
- StatNodeRef parent_ref = parent ? parent->ref : INVALID_STAT_REF;
+ StatNodeRef parent_ref = (parent ? parent->masked_ref : 0) ^ INVALID_STAT_REF;
counter->counter =
stat_file_add_counter(reds->stat_file, parent_ref, name, visible);
}
diff --git a/server/stat.h b/server/stat.h
index 5255efa..d191063 100644
--- a/server/stat.h
+++ b/server/stat.h
@@ -32,7 +32,10 @@ typedef struct {
typedef struct {
#ifdef RED_STATISTICS
- uint32_t ref;
+ /* to allow the use of 0 as default invalid value (ie memset(0)
+ * to work) this value is "masked" with INVALID_STAT_REF
+ * (which is part of file ABI so cannot be changes) */
+ uint32_t masked_ref;
#endif
} RedStatNode;
Does it make sense?
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel