From 92d0297e1e4d5946c5b098e37c91c7e524a0eca0 Mon Sep 17 00:00:00 2001 From: Jim Warner Date: Mon, 10 Aug 2020 00:00:00 -0500 Subject: [PATCH] library: introduced 'Item_table' validation provisions The recent work on updating the and 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 --- proc/diskstats.c | 36 +++++++++++++++++++++++++++++++++--- proc/meminfo.c | 35 ++++++++++++++++++++++++++++++++--- proc/pids.c | 35 ++++++++++++++++++++++++++++++++--- proc/slabinfo.c | 45 +++++++++++++++++++++++++++++++++++++-------- proc/stat.c | 35 ++++++++++++++++++++++++++++++++--- proc/vmstat.c | 36 +++++++++++++++++++++++++++++++++--- 6 files changed, 199 insertions(+), 23 deletions(-) diff --git a/proc/diskstats.c b/proc/diskstats.c index 10bf5fe9..3f9ff7c4 100644 --- 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 0a239552..2ae28763 100644 --- 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 61fb5d17..a17245d0 100644 --- 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 2bdb663f..e3b7598a 100644 --- 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 9c5b2012..110932ad 100644 --- 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 c9b8d826..331659a4 100644 --- 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)))) -- 2.40.0