Make RedStatFile private inside stat-file.c

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>

server/reds-private.h

@@ -121,7 +121,7 @@ struct RedsState {
     SSL_CTX *ctx;
 
 #ifdef RED_STATISTICS
-    RedStatFile stat_file;
+    RedStatFile *stat_file;
 #endif
     int allow_multiple_clients;
 

server/reds.c

@@ -352,22 +352,22 @@ static void reds_link_free(RedLinkInfo *link)
 
 StatNodeRef stat_add_node(RedsState *reds, StatNodeRef parent, const char *name, int visible)
 {
-    return stat_file_add_node(&reds->stat_file, parent, name, visible);
+    return stat_file_add_node(reds->stat_file, parent, name, visible);
 }
 
 void stat_remove_node(RedsState *reds, StatNodeRef ref)
 {
-    stat_file_remove_node(&reds->stat_file, ref);
+    stat_file_remove_node(reds->stat_file, ref);
 }
 
 uint64_t *stat_add_counter(RedsState *reds, StatNodeRef parent, const char *name, int visible)
 {
-    return stat_file_add_counter(&reds->stat_file, parent, name, visible);
+    return stat_file_add_counter(reds->stat_file, parent, name, visible);
 }
 
 void stat_remove_counter(RedsState *reds, uint64_t *counter)
 {
-    stat_file_remove_counter(&reds->stat_file, counter);
+    stat_file_remove_counter(reds->stat_file, counter);
 }
 #endif
 
@@ -2827,7 +2827,7 @@ static int reds_init_ssl(RedsState *reds)
 static void reds_cleanup(RedsState *reds)
 {
 #ifdef RED_STATISTICS
-    stat_file_unlink(&reds->stat_file);
+    stat_file_unlink(reds->stat_file);
 #endif
 }
 
@@ -3359,7 +3359,7 @@ static int do_spice_init(RedsState *reds, SpiceCoreInterface *core_interface)
     }
 
 #ifdef RED_STATISTICS
-    stat_file_init(&reds->stat_file, REDS_MAX_STAT_NODES);
+    reds->stat_file = stat_file_new(REDS_MAX_STAT_NODES);
 #endif
 
     if (reds_init_net(reds) < 0) {

server/stat-file.c

@@ -34,25 +34,36 @@
 #define STAT_SHM_SIZE(max_nodes) \
     (sizeof(SpiceStat) + (max_nodes) * sizeof(SpiceStatNode))
 
-void stat_file_init(RedStatFile *stat_file, unsigned int max_nodes)
+struct RedStatFile {
+    char *shm_name;
+    SpiceStat *stat;
+    pthread_mutex_t lock;
+    unsigned int max_nodes;
+};
+
+RedStatFile *stat_file_new(unsigned int max_nodes)
 {
     int fd;
     size_t shm_size = STAT_SHM_SIZE(max_nodes);
+    RedStatFile *stat_file = spice_new0(RedStatFile, 1);
 
     stat_file->max_nodes = max_nodes;
     stat_file->shm_name = g_strdup_printf(SPICE_STAT_SHM_NAME, getpid());
     shm_unlink(stat_file->shm_name);
     if ((fd = shm_open(stat_file->shm_name, O_CREAT | O_RDWR, 0444)) == -1) {
         spice_error("statistics shm_open failed, %s", strerror(errno));
+        goto cleanup;
     }
     if (ftruncate(fd, shm_size) == -1) {
         close(fd);
         spice_error("statistics ftruncate failed, %s", strerror(errno));
+        goto cleanup;
     }
     stat_file->stat = (SpiceStat *)mmap(NULL, shm_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
     close(fd);
     if (stat_file->stat == (SpiceStat *)MAP_FAILED) {
         spice_error("statistics mmap failed, %s", strerror(errno));
+        goto cleanup;
     }
     memset(stat_file->stat, 0, shm_size);
     stat_file->stat->magic = SPICE_STAT_MAGIC;
@@ -60,7 +71,18 @@ void stat_file_init(RedStatFile *stat_file, unsigned int max_nodes)
     stat_file->stat->root_index = INVALID_STAT_REF;
     if (pthread_mutex_init(&stat_file->lock, NULL)) {
         spice_error("mutex init failed");
+        goto cleanup;
     }
+    return stat_file;
+
+cleanup:
+    free(stat_file);
+    return NULL;
+}
+
+const char *stat_file_get_shm_name(RedStatFile *stat_file)
+{
+    return stat_file->shm_name;
 }
 
 void stat_file_unlink(RedStatFile *stat_file)

server/stat-file.h

@@ -24,15 +24,11 @@
 typedef uint32_t StatNodeRef;
 #define INVALID_STAT_REF (~(StatNodeRef)0)
 
-typedef struct {
-    char *shm_name;
-    SpiceStat *stat;
-    pthread_mutex_t lock;
-    unsigned int max_nodes;
-} RedStatFile;
+typedef struct RedStatFile RedStatFile;
 
-void stat_file_init(RedStatFile *stat_file, unsigned int max_nodes);
+RedStatFile *stat_file_new(unsigned int max_nodes);
 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,

server/tests/stat-file-test.c

@@ -34,30 +34,31 @@
 
 static void stat_file(void)
 {
-    RedStatFile stat_file;
+    RedStatFile *stat_file;
     StatNodeRef ref, refs[10];
-    uint64_t *counter, *counters[10];
+    uint64_t *counter, *counters[10] SPICE_GNUC_UNUSED;
     int i;
     char *filename = NULL;
     char name[20];
 
     /* create */
-    stat_file_init(&stat_file, 10);
+    stat_file = stat_file_new(10);
 
-    g_assert_nonnull(stat_file.shm_name);
-    filename = strdup(stat_file.shm_name);
-    g_assert(access(stat_file.shm_name, R_OK));
+    g_assert_nonnull(stat_file);
+    g_assert_nonnull(stat_file_get_shm_name(stat_file));
+    filename = strdup(stat_file_get_shm_name(stat_file));
+    g_assert(access(filename, R_OK));
 
     /* fill all nodes */
     for (i = 0; i < 10; ++i) {
         sprintf(name, "node %d", i);
-        ref = stat_file_add_node(&stat_file, INVALID_STAT_REF, name, TRUE);
+        ref = stat_file_add_node(stat_file, INVALID_STAT_REF, name, TRUE);
         refs[i] = ref;
         g_assert_cmpuint(ref,!=,INVALID_STAT_REF);
     }
 
     /* should fail */
-    ref = stat_file_add_node(&stat_file, INVALID_STAT_REF, "invalid", TRUE);
+    ref = stat_file_add_node(stat_file, INVALID_STAT_REF, "invalid", TRUE);
     g_assert_cmpuint(ref,==,INVALID_STAT_REF);
 
     /* we should find already present nodes */
@@ -66,7 +67,7 @@ static void stat_file(void)
          * As 17 and 10 are coprime numbers you'll get the same numbers
          * after 10 iterations */
         sprintf(name, "node %d", (i * 17 + 5) % 10);
-        ref = stat_file_add_node(&stat_file, INVALID_STAT_REF, name, TRUE);
+        ref = stat_file_add_node(stat_file, INVALID_STAT_REF, name, TRUE);
         g_assert_cmpuint(ref,!=,INVALID_STAT_REF);
     }
 
@@ -74,22 +75,22 @@ static void stat_file(void)
     for (i = 0; i < 6; ++i) {
         /* see above why the formula is used */
         int n = (i * 23 + 3) % 10;
-        stat_file_remove_node(&stat_file, refs[n]);
+        stat_file_remove_node(stat_file, refs[n]);
         refs[n] = INVALID_STAT_REF;
     }
 
     /* now there should be some place for some counters */
     for (i = 0; i < 6; ++i) {
         sprintf(name, "counter %d", i);
-        counter = stat_file_add_counter(&stat_file, INVALID_STAT_REF, name, TRUE);
+        counter = stat_file_add_counter(stat_file, INVALID_STAT_REF, name, TRUE);
         counters[i] = counter;
         g_assert_nonnull(counter);
     }
-    counter = stat_file_add_counter(&stat_file, INVALID_STAT_REF, "invalid", TRUE);
+    counter = stat_file_add_counter(stat_file, INVALID_STAT_REF, "invalid", TRUE);
     g_assert_null(counter);
 
-    stat_file_unlink(&stat_file);
-    g_assert_null(stat_file.shm_name);
+    stat_file_unlink(stat_file);
+    g_assert_null(stat_file_get_shm_name(stat_file));
     g_assert_cmpint(access(filename, F_OK),==,-1);
     free(filename);
 }