[Spice-devel] stat file and API

[Frediano Ziglio]

Hi,
  as it looks somebody is touching this part of code.

Looking at APIs (stat-file.h) we have these functions:

RedStatFile *stat_file_new(unsigned int max_nodes);
void stat_file_free(RedStatFile *stat_file);
void stat_file_unlink(RedStatFile *file_stat);
const char *stat_file_get_shm_name(RedStatFile *stat_file);
StatNodeRef stat_file_add_node(RedStatFile *stat_file, StatNodeRef parent,
                               const char *name, int visible);
uint64_t *stat_file_add_counter(RedStatFile *stat_file, StatNodeRef parent,
                                const char *name, int visible);
void stat_file_remove_node(RedStatFile *stat_file, StatNodeRef ref);
void stat_file_remove_counter(RedStatFile *stat_file, uint64_t *counter);


Currently no code calls the stat_file_remove_* functions (beside the test).


There is a problem adding/removing nodes/counters. 2 calls with same
parent+name retry the same node (a counter is implemented as a node) but
as there is no reference counting on nodes the first remove call will remove
the node despite the second reference so you'll have an invalid reference
to a deleted node.
This is not a problem as I stated before remove functions are not called
but what's the point in some API that cannot safely called? They should
either be removed or fixed.
As the file is basically an array of nodes (+ header) we could allocate
an array of reference counters inside RedStatFile and use it (the file
is used only by a single process for writing).


>From the API prospective the code is not much reusable as the filename
used is basically fixed as

   g_strdup_printf(SPICE_STAT_SHM_NAME, getpid());

this can easily be fixed passing a filename parameter to stat_file_new
(if we care about code reuse).

Frediano