library: introduced 'Item_table' validation provisions

The recent work on updating the <meminfo> and <vmstat>
modules with some newly added linux fields reminded me
(again) of a need for some mechanism guaranteeing that
a header file agrees with the source file assumptions.

Sadly, in the past, if a table entry was omitted or if
the table and header are ordered differently, then the
library would silently return the wrong results values
or even potentially experience a SIGSEGV abnormal end.

This patch offers a much needed development assist for
ensuring that Item_table entries are synchronized with
header file enumerators in terms of number plus order.

It's intended solely for our use as libprocps evolves.

Now, by activating ITEMTABLE_DEBUG, either directly or
via ./configure CFLAGS='-DITEMTABLE_DEBUG', the number
and order will be verified. It is envisioned that this
feature will be used at least once prior to a release.

Signed-off-by: Jim Warner <james.warner@comcast.net>

diff --git a/proc/diskstats.c b/proc/diskstats.c
index 10bf5fe933e3844621951d13b8165059d1aca496..3f9ff7c43e79037918f95353b4aafb8267529278 100644 (file)
--- a/proc/diskstats.c
+++ b/proc/diskstats.c
@@ -50,6 +50,14 @@
 #define STACKS_INCR         64           // amount reap stack allocations grow
 #define STR_COMPARE         strverscmp
 
+/* ----------------------------------------------------------------------- +
+   this provision can help ensure that our Item_table remains synchronized |
+   with the enumerators found in the associated header file. It's intended |
+   to only be used locally (& temporarily) at some point before a release! | */
+// #define ITEMTABLE_DEBUG //--------------------------------------------- |
+// ----------------------------------------------------------------------- +
+
+
 struct dev_data {
     unsigned long reads;
     unsigned long reads_merged;
@@ -200,7 +208,11 @@ srtDECL(noop) { \
 // ___ Controlling Table ||||||||||||||||||||||||||||||||||||||||||||||||||||||
 
 typedef void (*SET_t)(struct diskstats_result *, struct dev_node *);
+#ifdef ITEMTABLE_DEBUG
+#define RS(e) (SET_t)setNAME(e), DISKSTATS_ ## e, STRINGIFY(DISKSTATS_ ## e)
+#else
 #define RS(e) (SET_t)setNAME(e)
+#endif
 
 typedef int  (*QSR_t)(const void *, const void *, void *);
 #define QS(t) (QSR_t)srtNAME(t)
@@ -214,6 +226,10 @@ typedef int  (*QSR_t)(const void *, const void *, void *);
          * those *enum diskstats_item* guys ! */
 static struct {
     SET_t setsfunc;              // the actual result setting routine
+#ifdef ITEMTABLE_DEBUG
+    int   enumnumb;              // enumerator (must match position!)
+    char *enum2str;              // enumerator name as a char* string
+#endif
     QSR_t sortfunc;              // sort cmp func for a specific type
     char *type2str;              // the result type as a string value
 } Item_table[] = {
@@ -250,9 +266,6 @@ static struct {
   { RS(DELTA_WRITE_TIME),     QS(s_int),   TS(s_int)  },
   { RS(DELTA_IO_TIME),        QS(s_int),   TS(s_int)  },
   { RS(DELTA_WEIGHTED_TIME),  QS(s_int),   TS(s_int)  },
-
- // dummy entry corresponding to DISKSTATS_logical_end ...
-  { NULL,                               NULL,        NULL       }
 };
 
     /* please note,
@@ -703,6 +716,23 @@ PROCPS_EXPORT int procps_diskstats_new (
 {
     struct diskstats_info *p;
 
+#ifdef ITEMTABLE_DEBUG
+    int i, failed = 0;
+    for (i = 0; i < MAXTABLE(Item_table); i++) {
+        if (i != Item_table[i].enumnumb) {
+            fprintf(stderr, "%s: enum/table error: Item_table[%d] was %s, but its value is %d\n"
+                , __FILE__, i, Item_table[i].enum2str, Item_table[i].enumnumb);
+            failed = 1;
+        }
+    }
+    if (i != DISKSTATS_logical_end) {
+        fprintf(stderr, "%s: DISKSTATS_logical_end is %d, expected %d\n"
+            , __FILE__, DISKSTATS_logical_end, i);
+        failed = 1;
+    }
+    if (failed) _Exit(EXIT_FAILURE);
+#endif
+
     if (info == NULL)
         return -EINVAL;
     if (!(p = calloc(1, sizeof(struct diskstats_info))))

diff --git a/proc/meminfo.c b/proc/meminfo.c
index 0a2395525236d7cdc5bacbd5a023e675ff42e9c7..2ae287631bb4abb516682852d34a1a23336883eb 100644 (file)
--- a/proc/meminfo.c
+++ b/proc/meminfo.c
@@ -35,6 +35,13 @@
 #define MEMINFO_FILE  "/proc/meminfo"
 #define MEMINFO_BUFF  8192
 
+/* ------------------------------------------------------------------------- +
+   this provision can be used to ensure that our Item_table was synchronized |
+   with those enumerators found in the associated header file. It's intended |
+   to only be used locally (& temporarily) at some point prior to a release! | */
+// #define ITEMTABLE_DEBUG //----------------------------------------------- |
+// ------------------------------------------------------------------------- +
+
 
 struct meminfo_data {
     unsigned long Active;
@@ -284,7 +291,11 @@ HST_set(SWAP_DELTA_USED,         s_int,  derived_swap_used)
 // ___ Controlling Table ||||||||||||||||||||||||||||||||||||||||||||||||||||||
 
 typedef void (*SET_t)(struct meminfo_result *, struct mem_hist *);
+#ifdef ITEMTABLE_DEBUG
+#define RS(e) (SET_t)setNAME(e), MEMINFO_ ## e, STRINGIFY(MEMINFO_ ## e)
+#else
 #define RS(e) (SET_t)setNAME(e)
+#endif
 
 #define TS(t) STRINGIFY(t)
 #define TS_noop ""
@@ -295,6 +306,10 @@ typedef void (*SET_t)(struct meminfo_result *, struct mem_hist *);
          * those 'enum meminfo_item' guys ! */
 static struct {
     SET_t setsfunc;              // the actual result setting routine
+#ifdef ITEMTABLE_DEBUG
+    int   enumnumb;              // enumerator (must match position!)
+    char *enum2str;              // enumerator name as a char* string
+#endif
     char *type2str;              // the result type as a string value
 } Item_table[] = {
 /*  setsfunc                   type2str
@@ -435,9 +450,6 @@ static struct {
   { RS(SWAP_DELTA_FREE),       TS(s_int)  },
   { RS(SWAP_DELTA_TOTAL),      TS(s_int)  },
   { RS(SWAP_DELTA_USED),       TS(s_int)  },
-
- // dummy entry corresponding to MEMINFO_logical_end ...
-  { NULL,                      NULL       }
 };
 
     /* please note,
@@ -787,6 +799,23 @@ PROCPS_EXPORT int procps_meminfo_new (
 {
     struct meminfo_info *p;
 
+#ifdef ITEMTABLE_DEBUG
+    int i, failed = 0;
+    for (i = 0; i < MAXTABLE(Item_table); i++) {
+        if (i != Item_table[i].enumnumb) {
+            fprintf(stderr, "%s: enum/table error: Item_table[%d] was %s, but its value is %d\n"
+                , __FILE__, i, Item_table[i].enum2str, Item_table[i].enumnumb);
+            failed = 1;
+        }
+    }
+    if (i != MEMINFO_logical_end) {
+        fprintf(stderr, "%s: MEMINFO_logical_end is %d, expected %d\n"
+            , __FILE__, MEMINFO_logical_end, i);
+        failed = 1;
+    }
+    if (failed) _Exit(EXIT_FAILURE);
+#endif
+
     if (info == NULL || *info != NULL)
         return -EINVAL;
     if (!(p = calloc(1, sizeof(struct meminfo_info))))

diff --git a/proc/pids.c b/proc/pids.c
index 61fb5d17932035e1da83e088fa32154cb7f7d293..a17245d038df465bcef148dc89bab962a58f364f 100644 (file)
--- a/proc/pids.c
+++ b/proc/pids.c
@@ -52,6 +52,13 @@
 #define STACKS_INCR  128               // amount reap stack allocations grow
 #define NEWOLD_INCR  128               // amt by which hist allocations grow
 
+/* ------------------------------------------------------------------------- +
+   this provision can be used to ensure that our Item_table was synchronized |
+   with those enumerators found in the associated header file. It's intended |
+   to only be used locally (& temporarily) at some point prior to a release! | */
+// #define ITEMTABLE_DEBUG //----------------------------------------------- |
+// ------------------------------------------------------------------------- +
+
 
 struct stacks_extent {
     int ext_numstacks;
@@ -357,7 +364,11 @@ typedef void (*SET_t)(struct pids_info *, struct pids_result *, proc_t *);
 typedef void (*FRE_t)(struct pids_result *);
 typedef int  (*QSR_t)(const void *, const void *, void *);
 
+#ifdef ITEMTABLE_DEBUG
+#define RS(e) (SET_t)setNAME(e), PIDS_ ## e, STRINGIFY(PIDS_ ## e)
+#else
 #define RS(e) (SET_t)setNAME(e)
+#endif
 #define FF(t) (FRE_t)freNAME(t)
 #define QS(t) (QSR_t)srtNAME(t)
 #define TS(t) STRINGIFY(t)
@@ -369,6 +380,10 @@ typedef int  (*QSR_t)(const void *, const void *, void *);
          * those 'enum pids_item' guys ! */
 static struct {
     SET_t    setsfunc;            // the actual result setting routine
+#ifdef ITEMTABLE_DEBUG
+    int      enumnumb;            // enumerator (must match position!)
+    char    *enum2str;            // enumerator name as a char* string
+#endif
     unsigned oldflags;            // PROC_FILLxxxx flags for this item
     FRE_t    freefunc;            // free function for strings storage
     QSR_t    sortfunc;            // sort cmp func for a specific type
@@ -499,9 +514,6 @@ static struct {
     { RS(VM_USED),           f_status,   NULL,      QS(ul_int),    0,        TS(ul_int)  },
     { RS(VSIZE_PGS),         f_stat,     NULL,      QS(ul_int),    0,        TS(ul_int)  },
     { RS(WCHAN_NAME),        0,          FF(str),   QS(str),       0,        TS(str)     }, // oldflags: tid already free
-
-   // dummy entry corresponding to PIDS_logical_end ...
-    { NULL,                  0,          NULL,      NULL,          0,        NULL        }
 };
 
     /* please note,
@@ -1116,6 +1128,23 @@ PROCPS_EXPORT int procps_pids_new (
     double uptime_secs;
     int pgsz;
 
+#ifdef ITEMTABLE_DEBUG
+    int i, failed = 0;
+    for (i = 0; i < MAXTABLE(Item_table); i++) {
+        if (i != Item_table[i].enumnumb) {
+            fprintf(stderr, "%s: enum/table error: Item_table[%d] was %s, but its value is %d\n"
+                , __FILE__, i, Item_table[i].enum2str, Item_table[i].enumnumb);
+            failed = 1;
+        }
+    }
+    if (i != PIDS_logical_end) {
+        fprintf(stderr, "%s: PIDS_logical_end is %d, expected %d\n"
+            , __FILE__, PIDS_logical_end, i);
+        failed = 1;
+    }
+    if (failed) _Exit(EXIT_FAILURE);
+#endif
+
     if (info == NULL || *info != NULL)
         return -EINVAL;
     if (!(p = calloc(1, sizeof(struct pids_info))))

diff --git a/proc/slabinfo.c b/proc/slabinfo.c
index 2bdb663f982afd367d92c6387773a644b669233d..e3b7598a08ab04717bdfef178cb81aaf6c61a572 100644 (file)
--- a/proc/slabinfo.c
+++ b/proc/slabinfo.c
@@ -47,6 +47,13 @@
 
 #define STACKS_INCR          128         // amount reap stack allocations grow
 
+/* ---------------------------------------------------------------------------- +
+   this #define will be used to help ensure that our Item_table is synchronized |
+   with all the enumerators found in the associated header file. It is intended |
+   to only be defined locally (and temporarily) at some point prior to release! | */
+// #define ITEMTABLE_DEBUG //-------------------------------------------------- |
+// ---------------------------------------------------------------------------- +
+
 /*
    Because 'select' could, at most, return only node[0] values and since 'reap' |
    would be forced to duplicate global slabs stuff in every node results stack, |
@@ -134,7 +141,7 @@ struct slabinfo_info {
 
 // ___ Results 'Set' Support ||||||||||||||||||||||||||||||||||||||||||||||||||
 
-#define setNAME(e) set_slabinfo_ ## e
+#define setNAME(e) set_ ## e
 #define setDECL(e) static void setNAME(e) \
     (struct slabinfo_result *R, struct slabs_hist *S, struct slabs_node *N)
 
@@ -144,8 +151,8 @@ struct slabinfo_info {
 // delta assignment
 #define HST_set(e,t,x) setDECL(e) { (void)N; R->result. t = (signed long)S->new. x - S->old. x; }
 
-setDECL(noop)  { (void)R; (void)S; (void)N; }
-setDECL(extra) { (void)S; (void)N; R->result.ul_int = 0; }
+setDECL(SLABINFO_noop)  { (void)R; (void)S; (void)N; }
+setDECL(SLABINFO_extra) { (void)S; (void)N; R->result.ul_int = 0; }
 
 NOD_set(SLAB_NAME,                     str,  name)
 NOD_set(SLAB_NUM_OBJS,               u_int,  nr_objs)
@@ -234,7 +241,11 @@ srtDECL(noop) { \
 // ___ Controlling Table ||||||||||||||||||||||||||||||||||||||||||||||||||||||
 
 typedef void (*SET_t)(struct slabinfo_result *, struct slabs_hist *, struct slabs_node *);
+#ifdef ITEMTABLE_DEBUG
+#define RS(e) (SET_t)setNAME(e), e, STRINGIFY(e)
+#else
 #define RS(e) (SET_t)setNAME(e)
+#endif
 
 typedef int  (*QSR_t)(const void *, const void *, void *);
 #define QS(t) (QSR_t)srtNAME(t)
@@ -248,13 +259,17 @@ typedef int  (*QSR_t)(const void *, const void *, void *);
          * those *enum slabinfo_item* guys ! */
 static struct {
     SET_t setsfunc;              // the actual result setting routine
+#ifdef ITEMTABLE_DEBUG
+    int   enumnumb;              // enumerator (must match position!)
+    char *enum2str;              // enumerator name as a char* string
+#endif
     QSR_t sortfunc;              // sort cmp func for a specific type
     char *type2str;              // the result type as a string value
 } Item_table[] = {
 /*  setsfunc                        sortfunc     type2str
     ------------------------------  -----------  ---------- */
-  { RS(noop),                       QS(noop),    TS_noop    },
-  { RS(extra),                      QS(ul_int),  TS_noop    },
+  { RS(SLABINFO_noop),              QS(noop),    TS_noop    },
+  { RS(SLABINFO_extra),             QS(ul_int),  TS_noop    },
 
   { RS(SLAB_NAME),                  QS(str),     TS(str)    },
   { RS(SLAB_NUM_OBJS),              QS(u_int),   TS(u_int)  },
@@ -292,9 +307,6 @@ static struct {
   { RS(SLABS_DELTA_PAGES_TOTAL),    QS(noop),    TS(s_int)  },
   { RS(SLABS_DELTA_SIZE_ACTIVE),    QS(noop),    TS(s_int)  },
   { RS(SLABS_DELTA_SIZE_TOTAL),     QS(noop),    TS(s_int)  },
-
- // dummy entry corresponding to SLABINFO_logical_end ...
-  { NULL,                           NULL,        NULL       }
 };
 
     /* please note,
@@ -732,6 +744,23 @@ PROCPS_EXPORT int procps_slabinfo_new (
 {
     struct slabinfo_info *p;
 
+#ifdef ITEMTABLE_DEBUG
+    int i, failed = 0;
+    for (i = 0; i < MAXTABLE(Item_table); i++) {
+        if (i != Item_table[i].enumnumb) {
+            fprintf(stderr, "%s: enum/table error: Item_table[%d] was %s, but its value is %d\n"
+                , __FILE__, i, Item_table[i].enum2str, Item_table[i].enumnumb);
+            failed = 1;
+        }
+    }
+    if (i != SLABINFO_logical_end) {
+        fprintf(stderr, "%s: SLABINFO_logical_end is %d, expected %d\n"
+            , __FILE__, SLABINFO_logical_end, i);
+        failed = 1;
+    }
+    if (failed) _Exit(EXIT_FAILURE);
+#endif
+
     if (info == NULL || *info != NULL)
         return -EINVAL;
     if (!(p = calloc(1, sizeof(struct slabinfo_info))))

diff --git a/proc/stat.c b/proc/stat.c
index 9c5b2012e71e345a8dff63703694c8ba3462d3aa..110932ad7206430b2bab3ae9c8405c970a035c32 100644 (file)
--- a/proc/stat.c
+++ b/proc/stat.c
@@ -39,6 +39,13 @@
 #define STACKS_INCR   64               // amount reap stack allocations grow
 #define NEWOLD_INCR   64               // amount jiffs hist allocations grow
 
+/* ------------------------------------------------------------------------- +
+   this provision can be used to ensure that our Item_table was synchronized |
+   with those enumerators found in the associated header file. It's intended |
+   to only be used locally (& temporarily) at some point prior to a release! | */
+// #define ITEMTABLE_DEBUG //----------------------------------------------- |
+// ------------------------------------------------------------------------- +
+
 /* ------------------------------------------------------------------------- +
    because 'reap' would be forced to duplicate the global SYS stuff in every |
    TIC type results stack, the following #define can be used to enforce that |
@@ -267,7 +274,11 @@ srtDECL(noop) { \
 // ___ Controlling Table ||||||||||||||||||||||||||||||||||||||||||||||||||||||
 
 typedef void (*SET_t)(struct stat_result *, struct hist_sys *, struct hist_tic *);
+#ifdef ITEMTABLE_DEBUG
+#define RS(e) (SET_t)setNAME(e), STAT_ ## e, STRINGIFY(STAT_ ## e)
+#else
 #define RS(e) (SET_t)setNAME(e)
+#endif
 
 typedef int  (*QSR_t)(const void *, const void *, void *);
 #define QS(t) (QSR_t)srtNAME(t)
@@ -281,6 +292,10 @@ typedef int  (*QSR_t)(const void *, const void *, void *);
          * those 'enum stat_item' guys ! */
 static struct {
     SET_t setsfunc;              // the actual result setting routine
+#ifdef ITEMTABLE_DEBUG
+    int   enumnumb;              // enumerator (must match position!)
+    char *enum2str;              // enumerator name as a char* string
+#endif
     QSR_t sortfunc;              // sort cmp func for a specific type
     char *type2str;              // the result type as a string value
 } Item_table[] = {
@@ -338,9 +353,6 @@ static struct {
   { RS(SYS_DELTA_PROC_BLOCKED),  QS(s_int),    TS(s_int)   },
   { RS(SYS_DELTA_PROC_CREATED),  QS(s_int),    TS(s_int)   },
   { RS(SYS_DELTA_PROC_RUNNING),  QS(s_int),    TS(s_int)   },
-
- // dummy entry corresponding to STAT_logical_end ...
-  { NULL,                        NULL,         NULL        }
 };
 
     /* please note,
@@ -854,6 +866,23 @@ PROCPS_EXPORT int procps_stat_new (
 {
     struct stat_info *p;
 
+#ifdef ITEMTABLE_DEBUG
+    int i, failed = 0;
+    for (i = 0; i < MAXTABLE(Item_table); i++) {
+        if (i != Item_table[i].enumnumb) {
+            fprintf(stderr, "%s: enum/table error: Item_table[%d] was %s, but its value is %d\n"
+                , __FILE__, i, Item_table[i].enum2str, Item_table[i].enumnumb);
+            failed = 1;
+        }
+    }
+    if (i != STAT_logical_end) {
+        fprintf(stderr, "%s: STAT_logical_end is %d, expected %d\n"
+            , __FILE__, STAT_logical_end, i);
+        failed = 1;
+    }
+    if (failed) _Exit(EXIT_FAILURE);
+#endif
+
     if (info == NULL || *info != NULL)
         return -EINVAL;
     if (!(p = calloc(1, sizeof(struct stat_info))))

diff --git a/proc/vmstat.c b/proc/vmstat.c
index c9b8d826a83da1e9cf09e1627fca5fdd6b66bf37..331659a427911bfd9f69426e7fe605425b962db0 100644 (file)
--- a/proc/vmstat.c
+++ b/proc/vmstat.c
@@ -41,6 +41,14 @@
 #define VMSTAT_FILE  "/proc/vmstat"
 #define VMSTAT_BUFF  8192
 
+/* ------------------------------------------------------------- +
+   this provision can be used to help ensure that our Item_table |
+   was synchronized with the enumerators found in the associated |
+   header file. It's intended to be used locally (& temporarily) |
+   at least once at some point prior to publishing new releases! | */
+// #define ITEMTABLE_DEBUG //----------------------------------- |
+// ------------------------------------------------------------- +
+
 /*
  *  Perhaps someday we'll all learn what is in these fields. But |
  *  that might require available linux documentation progressing |
@@ -557,7 +565,11 @@ HST_set(DELTA_ZONE_RECLAIM_FAILED,             zone_reclaim_failed)
 // ___ Controlling Table ||||||||||||||||||||||||||||||||||||||||||||||||||||||
 
 typedef void (*SET_t)(struct vmstat_result *, struct vmstat_hist *);
+#ifdef ITEMTABLE_DEBUG
+#define RS(e) (SET_t)setNAME(e), VMSTAT_ ## e, STRINGIFY(VMSTAT_ ## e)
+#else
 #define RS(e) (SET_t)setNAME(e)
+#endif
 
 #define TS(t) STRINGIFY(t)
 #define TS_noop ""
@@ -568,6 +580,10 @@ typedef void (*SET_t)(struct vmstat_result *, struct vmstat_hist *);
          * those 'enum vmstat_item' guys ! */
 static struct {
     SET_t setsfunc;              // the actual result setting routine
+#ifdef ITEMTABLE_DEBUG
+    int   enumnumb;              // enumerator (must match position!)
+    char *enum2str;              // enumerator name as a char* string
+#endif
     char *type2str;              // the result type as a string value
 } Item_table[] = {
 /*  setsfunc                                   type2str
@@ -878,9 +894,6 @@ static struct {
   { RS(DELTA_WORKINGSET_REFAULT),              TS(sl_int) },
   { RS(DELTA_WORKINGSET_RESTORE),              TS(sl_int) },
   { RS(DELTA_ZONE_RECLAIM_FAILED),             TS(sl_int) },
-
- // dummy entry corresponding to VMSTAT_logical_end ...
-  { NULL,                                      NULL       }
 };
 
     /* please note,
@@ -1283,6 +1296,23 @@ PROCPS_EXPORT int procps_vmstat_new (
 {
     struct vmstat_info *p;
 
+#ifdef ITEMTABLE_DEBUG
+    int i, failed = 0;
+    for (i = 0; i < MAXTABLE(Item_table); i++) {
+        if (i != Item_table[i].enumnumb) {
+            fprintf(stderr, "%s: enum/table error: Item_table[%d] was %s, but its value is %d\n"
+                , __FILE__, i, Item_table[i].enum2str, Item_table[i].enumnumb);
+            failed = 1;
+        }
+    }
+    if (i != VMSTAT_logical_end) {
+        fprintf(stderr, "%s: VMSTAT_logical_end is %d, expected %d\n"
+            , __FILE__, VMSTAT_logical_end, i);
+        failed = 1;
+    }
+    if (failed) _Exit(EXIT_FAILURE);
+#endif
+
     if (info == NULL || *info != NULL)
         return -EINVAL;
     if (!(p = calloc(1, sizeof(struct vmstat_info))))