[Spice-devel] [PATCH spice-server v3] red-channel: Initialize statistic node correctly

[Christophe Fergeau]

On Thu, Mar 30, 2017 at 01:45:05PM +0100, Frediano Ziglio wrote:
> The default memset(0) on the node did not init it correctly.

"Doing a memset(0) on a SpiceStatNode does not create an empty/unattached
node, but instead creates a node whose parent is the node '0'. This node
could be non-existing, or totally unrelated to the empty node we wanted
to create."

> This patch create the 0 node as soon as the file is created
> to use as default memset(0) compatible node.

"This patch creates a default '0' node as soon as the file is created.
This will make the behaviour of 0-filled nodes more in line with what
one would expect."

I think the log becomes a lot clearer as to what you are fixing and what
the problem was with the suggested amendments.

Looks good to me, though I'm not overly familiar with the stat code.

Christophe

> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  server/reds.c                 |  4 ++++
>  server/tests/test-stat-file.c | 19 +++++++++++++++++++
>  2 files changed, 23 insertions(+)
> 
> Changes since v2:
> - this patch create an additional node making 0 a valid node.
>   This also make clear that the counters under it apply to a channel.
> 
> diff --git a/server/reds.c b/server/reds.c
> index 0b6ca12..839ffe7 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -3500,6 +3500,10 @@ SPICE_GNUC_VISIBLE SpiceServer *spice_server_new(void)
>      reds->config->exit_on_disconnect = FALSE;
>  #ifdef RED_STATISTICS
>      reds->stat_file = stat_file_new(REDS_MAX_STAT_NODES);
> +    /* Create an initial node. This will be the 0 node making easier
> +     * to initialize node references.
> +     */
> +    stat_file_add_node(reds->stat_file, INVALID_STAT_REF, "default_channel", TRUE);
>  #endif
>      reds->listen_socket = -1;
>      reds->secure_listen_socket = -1;
> diff --git a/server/tests/test-stat-file.c b/server/tests/test-stat-file.c
> index 63a2a2b..901985a 100644
> --- a/server/tests/test-stat-file.c
> +++ b/server/tests/test-stat-file.c
> @@ -89,11 +89,30 @@ static void stat_file(void)
>      stat_file_free(stat_file);
>  }
>  
> +/* make sure first node is node 0 */
> +static void stat_file_start(void)
> +{
> +    RedStatFile *stat_file;
> +    StatNodeRef ref;
> +
> +    /* create */
> +    stat_file = stat_file_new(10);
> +    g_assert_nonnull(stat_file);
> +
> +    ref = stat_file_add_node(stat_file, INVALID_STAT_REF, "ZERO", TRUE);
> +    g_assert_cmpuint(ref,==,0);
> +
> +    stat_file_unlink(stat_file);
> +    stat_file_free(stat_file);
> +}
> +
> +
>  int main(int argc, char *argv[])
>  {
>      g_test_init(&argc, &argv, NULL);
>  
>      g_test_add_func("/server/stat-file", stat_file);
> +    g_test_add_func("/server/stat-file-start", stat_file_start);
>  
>      return g_test_run();
>  }
> -- 
> 2.9.3
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170330/8ea99e89/attachment.sig>