]> granicus.if.org Git - procps-ng/commitdiff
library: final tweaks to code and/or comments, 3rd gen
authorJim Warner <james.warner@comcast.net>
Thu, 9 Jun 2016 05:00:00 +0000 (00:00 -0500)
committerCraig Small <csmall@dropbear.xyz>
Mon, 13 Jun 2016 00:33:37 +0000 (10:33 +1000)
With the dust now settling on all those 3rd generation
upgrades, this patch tries to provide some consistency
among the separate modules involved. Someday we should
consider a 4th generation where all redundant code has
been removed and isolated in a new shared source file.

Following is a summary of significant changes (if any)
to each of these now upgraded 3rd gen library modules.

<meminfo> ............................................
. strictly formatting/comment changes, code unaffected

<pids> ...............................................
. replaced a local mkSTR macro with existing STRINGIFY
. added fetch narrative explaining duplicate addresses

<slabinfo> ...........................................
. rearranged some free logic for procps_slabinfo_unref
. added fetch narrative explaining duplicate addresses

<stat> ...............................................
. added #define ENFORCE_LOGICAL, just as in <slabinfo>
. replaced a local mkSTR macro with existing STRINGIFY
. alphabetized the function declarations in the header

<vmstat> .............................................
. made one coverity concession with read_vmstat_failed

[ several of these changes may reflect this author's ]
[ continuing pursuit of an unreasonable goal -- that ]
[ of a 'perfect' (plus 'pretty') C language program! ]

Signed-off-by: Jim Warner <james.warner@comcast.net>
proc/meminfo.c
proc/meminfo.h
proc/pids.c
proc/pids.h
proc/slabinfo.c
proc/slabinfo.h
proc/stat.c
proc/stat.h
proc/vmstat.c
proc/vmstat.h

index 38dc8a628014bf850d612180cebe6a7ef1305b44..a2be2bec64d90a79db8c05a0b22b7ee17b167a18 100644 (file)
@@ -33,6 +33,7 @@
 
 #define MEMINFO_FILE "/proc/meminfo"
 
+
 struct meminfo_data {
 /** == man 5 proc shows these as: to be documented, so the hell with 'em */
     unsigned long Active;
@@ -445,6 +446,7 @@ static struct {
   { RS(SWAP_TOTAL),              RG(SWAP_TOTAL)               },
   { RS(SWAP_USED),               RG(SWAP_USED)                },
 
+ // dummy entry corresponding to PROCPS_MEMINFO_logical_end ...
   { NULL,                        NULL                         }
 };
 
index c7d148270a4b73f0b40aea647293e21f78bdc075..9343058c45422ef14d6c97bd1550997a61c10977 100644 (file)
@@ -22,8 +22,8 @@
 __BEGIN_DECLS
 
 enum meminfo_item {
-    PROCPS_MEMINFO_noop,                  // n/a     ( never altered )
-    PROCPS_MEMINFO_extra,                 // n/a     ( reset to zero )
+    PROCPS_MEMINFO_noop,                  //        ( never altered )
+    PROCPS_MEMINFO_extra,                 //        ( reset to zero )
     /*
         note: all of the following values are exressed as KiB
     */
index c575dbb0311a7c8edc1cad1e6174dbb5b3c729d7..7726ff90c24c0a08d8ddaaf69028186fabf9ae6f 100644 (file)
 #include <sys/stat.h>
 #include <sys/types.h>
 
-#include <proc/pids.h>
+#include <proc/devname.h>
+#include <proc/readproc.h>
 #include <proc/sysinfo.h>
 #include <proc/uptime.h>
-#include "procps-private.h"
+#include <proc/wchan.h>
+
+#include <proc/procps-private.h>
+#include <proc/pids.h>
 
-#include "devname.h"                   // and a few headers for our
-#include "readproc.h"                  // bridged libprocps support
-#include "wchan.h"                     // ( maybe just temporary? )
 
 //#define UNREF_RPTHASH                // report on hashing, at uref time
 
@@ -107,9 +108,6 @@ static char** vectorize_this (const char* src) {
 } // end: vectorize_this
 
 
-#define mkSTR(a) xySTR(a)
-#define xySTR(z) #z
-
 #define setNAME(e) set_results_ ## e
 #define setDECL(e) static void setNAME(e) \
     (struct procps_pidsinfo *I, struct pids_result *R, proc_t *P)
@@ -127,12 +125,12 @@ static char** vectorize_this (const char* src) {
    some sort of hint that they duplicated this char * item ... */
 #define STR_set(e,x) setDECL(e) { \
     (void)I; if (NULL != P-> x) { R->result.str = P-> x; P-> x = NULL; } \
-    else R->result.str = strdup("[ duplicate " mkSTR(e) " ]"); }
+    else R->result.str = strdup("[ duplicate " STRINGIFY(e) " ]"); }
 /* take ownership of true vectorized strings if possible, else return
    some sort of hint that they duplicated this char ** item ... */
 #define VEC_set(e,x) setDECL(e) { \
     (void)I; if (NULL != P-> x) { R->result.strv = P-> x;  P-> x = NULL; } \
-    else R->result.strv = vectorize_this("[ duplicate " mkSTR(e) " ]"); }
+    else R->result.strv = vectorize_this("[ duplicate " STRINGIFY(e) " ]"); }
 
 
 setDECL(noop)         { (void)I; (void)R; (void)P; return; }
@@ -254,9 +252,6 @@ REG_set(VSIZE_PGS,        ul_int,  vsize)
 REG_set(WCHAN_ADDR,       ul_int,  wchan)
 setDECL(WCHAN_NAME)   { (void)I; R->result.str = strdup(lookup_wchan(P->tid)); }
 
-#undef mkSTR
-#undef xySTR
-
 #undef setDECL
 #undef CVT_set
 #undef DUP_set
@@ -388,6 +383,7 @@ static struct {
       ---------------------  ----------  ---------  -------------  -------- */
     { RS(noop),              0,          NULL,      QS(noop),      0        }, // user only, never altered
     { RS(extra),             0,          NULL,      QS(ull_int),   0        }, // user only, reset to zero
+
     { RS(ADDR_END_CODE),     f_stat,     NULL,      QS(ul_int),    0        },
     { RS(ADDR_KSTK_EIP),     f_stat,     NULL,      QS(ul_int),    0        },
     { RS(ADDR_KSTK_ESP),     f_stat,     NULL,      QS(ul_int),    0        },
@@ -504,6 +500,7 @@ static struct {
     { RS(VSIZE_PGS),         f_stat,     NULL,      QS(ul_int),    0        },
     { RS(WCHAN_ADDR),        f_stat,     NULL,      QS(ul_int),    0        },
     { RS(WCHAN_NAME),        0,          FF(str),   QS(str),       0        }, // oldflags: tid already free
+
    // dummy entry corresponding to PROCPS_PIDS_logical_end ...
     { NULL,                  0,          NULL,      NULL,          0        }
 };
@@ -1094,6 +1091,9 @@ static int stacks_fetch (
     }
 
     // finalize stuff -------------------------------------
+    /* note: we go to this trouble of maintaining a duplicate of the consolidated |
+             extent stacks addresses represented as our 'anchor' since these ptrs |
+             are exposed to users ( um, not that we don't trust 'em or anything ) | */
     if (n_saved < n_alloc + 1) {
         n_saved = n_alloc + 1;
         if (!(info->fetch.results.stacks = realloc(info->fetch.results.stacks, sizeof(void *) * n_saved)))
index 56c586bc9af51a87d6a2feabbf809267ec364af2..c9838ccd7a4c75dde7dfa06369333bb06fa14f7e 100644 (file)
@@ -27,8 +27,8 @@
 __BEGIN_DECLS
 
 enum pids_item {
-    PROCPS_PIDS_noop,              //         ( never altered )
-    PROCPS_PIDS_extra,             //         ( reset to zero )
+    PROCPS_PIDS_noop,              //        ( never altered )
+    PROCPS_PIDS_extra,             //        ( reset to zero )
     PROCPS_PIDS_ADDR_END_CODE,     //  ul_int
     PROCPS_PIDS_ADDR_KSTK_EIP,     //  ul_int
     PROCPS_PIDS_ADDR_KSTK_ESP,     //  ul_int
index 684ece5b784a2e7d478b0c8c341ff1374de95bbe..4516c2325a553cf8f24c07bf1b9f7c45d2c1ed50 100644 (file)
 #include <sys/stat.h>
 #include <sys/types.h>
 
-#include "procps-private.h"
+#include <proc/procps-private.h>
 #include <proc/slabinfo.h>
 
+
 #define SLABINFO_FILE        "/proc/slabinfo"
 #define SLABINFO_LINE_LEN    2048
 #define SLAB_INFO_NAME_LEN   128
 
-
 /*
-   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,
-   the following #define can be used to enforce strictly logical return values.
+   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, |
+   the following #define can be used to enforce strictly logical return values. |
       select: allow only PROCPS_SLABINFO & PROCPS_SLABS items
       reap:   allow only PROCPS_SLABINFO & PROCPS_SLABNODE items
-   Without the #define, these functions always return something (even if just 0)
+   Without the #define, these functions always return something even if just 0. |
       get:    return only PROCPS_SLABS results, else 0
       select: return only PROCPS_SLABINFO & PROCPS_SLABS results, else zero
       reap:   return any requested, even when duplicated in each node's stack */
-//#define ENFORCE_LOGICAL
+//#define ENFORCE_LOGICAL  // ensure only logical items accepted by select/reap
 
 
 struct slabs_summ {
@@ -112,7 +112,7 @@ struct fetch_support {
     struct slabinfo_stack **anchor; // fetch consolidated extents
     int n_alloc;                    // number of above pointers allocated
     int n_inuse;                    // number of above pointers occupied
-    int n_alloc_save;               // last known reaped.stacks allocation
+    int n_alloc_save;               // last known reap.stacks allocation
     struct slabinfo_reap results;   // count + stacks for return to caller
 };
 
@@ -351,6 +351,7 @@ static struct {
   { RS(SLABNODE_USE),            RG(SLABNODE_USE),            QS(u_int)  },
   { RS(SLABNODE_SIZE),           RG(SLABNODE_SIZE),           QS(ul_int) },
 
+ // dummy entry corresponding to PROCPS_SLABINFO_logical_end ...
   { NULL,                        NULL,                        NULL       }
 };
 
@@ -781,6 +782,9 @@ static int stacks_fetch (
     }
 
     // finalize stuff -------------------------------------
+    /* note: we go to this trouble of maintaining a duplicate of the consolidated |
+             extent stacks addresses represented as our 'anchor' since these ptrs |
+             are exposed to users ( um, not that we don't trust 'em or anything ) | */
     if (n_saved < n_alloc + 1) {
         n_saved = n_alloc + 1;
         if (!(info->fetch.results.stacks = realloc(info->fetch.results.stacks, sizeof(void *) * n_saved)))
@@ -853,9 +857,9 @@ PROCPS_EXPORT int procps_slabinfo_new (
 
     p->refcount = 1;
 
-    /* do a priming read here for the following potential benefits:
-         1) see if the caller's permissions are sufficient (root)
-         2) make delta results potentially useful the first time  */
+    /* do a priming read here for the following potential benefits: |
+         1) see if that caller's permissions were sufficient (root) |
+         2) make delta results potentially useful, even is 1st time | */
     if ((rc = read_slabinfo_failed(p))) {
         procps_slabinfo_unref(&p);
         return rc;
@@ -889,16 +893,16 @@ PROCPS_EXPORT int procps_slabinfo_unref (
             fclose((*info)->slabinfo_fp);
             (*info)->slabinfo_fp = NULL;
         }
-        if ((*info)->fetch.anchor)
-            free((*info)->fetch.anchor);
-        if ((*info)->fetch.results.stacks)
-            free((*info)->fetch.results.stacks);
-
         if ((*info)->select_ext.extents)
             extents_free_all((&(*info)->select_ext));
         if ((*info)->select_ext.items)
             free((*info)->select_ext.items);
 
+        if ((*info)->fetch.anchor)
+            free((*info)->fetch.anchor);
+        if ((*info)->fetch.results.stacks)
+            free((*info)->fetch.results.stacks);
+
         if ((*info)->fetch_ext.extents)
             extents_free_all(&(*info)->fetch_ext);
         if ((*info)->fetch_ext.items)
@@ -943,8 +947,8 @@ PROCPS_EXPORT signed long procps_slabinfo_get (
 
 /* procps_slabinfo_reap():
  *
- * Harvest all the requested SLABNODE information providing the
- * result stacks along with totals via the reap summary.
+ * Harvest all the requested SLABNODE (individual nodes) information
+ * providing the result stacks along with the total number of nodes.
  *
  * Returns: pointer to a slabinfo_reap struct on success, NULL on error.
  */
@@ -973,8 +977,8 @@ PROCPS_EXPORT struct slabinfo_reap *procps_slabinfo_reap (
 
 /* procps_slabinfo_select():
  *
- * Harvest all the requested MEM and/or SWAP information then return
- * it in a results stack.
+ * Obtain all the requested SLABS (global) information then return
+ * it in a single library provided results stack.
  *
  * Returns: pointer to a slabinfo_stack struct on success, NULL on error.
  */
index f28471d631e6e3a110415fde1f2f18843962aee9..0c3df56fa6051efcd9365ecc2203a69b973ce6df 100644 (file)
@@ -28,8 +28,8 @@
 __BEGIN_DECLS
 
 enum slabinfo_item {
-    PROCPS_SLABINFO_noop,            //        ( never altered )
-    PROCPS_SLABINFO_extra,           //        ( reset to zero )
+    PROCPS_SLABINFO_noop,            //       ( never altered )
+    PROCPS_SLABINFO_extra,           //       ( reset to zero )
 
     PROCPS_SLABS_OBJS,               //  u_int
     PROCPS_SLABS_AOBJS,              //  u_int
index 8dde12337929fb6e5cffd4cb29e580208bd3622c..4f8ea3e27649bf3a39d60748b7cd8836ef137d75 100644 (file)
 #include <sys/stat.h>
 #include <sys/types.h>
 
-#include <proc/stat.h>
 #include <proc/sysinfo.h>
-#include "procps-private.h"
 
+#include <proc/procps-private.h>
+#include <proc/stat.h>
 
-/* -------------------------------------------------------------------------
-   strictly development define(s), largely for the top program
-   ( next has no affect if ./configure --disable-numa has been specified ) */
-//#define PRETEND_NUMA          // pretend there are 3 'discontiguous' nodes
-// -------------------------------------------------------------------------
 
 #define STAT_FILE "/proc/stat"
 
 #define STACKS_INCR   64               // amount reap stack allocations grow
 #define NEWOLD_INCR   32               // amount jiffs hist allocations grow
 
+/* ------------------------------------------------------------------------- +
+   a strictly development #define, existing specifically for the top program |
+   ( and it has no affect if ./configure --disable-numa has been specified ) | */
+//#define PRETEND_NUMA     // pretend there are 3 'discontiguous' numa nodes |
+// ------------------------------------------------------------------------- +
+
+/* ------------------------------------------------------------------------- +
+   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 |
+   only PROCPS_STAT_noop/extra plus those PROCPS_STAT_TIC items were allowed | */
+//#define ENFORCE_LOGICAL  // ensure only logical items are accepted by reap |
+// ------------------------------------------------------------------------- +
+
 
 struct stat_jifs {
     unsigned long long user, nice, system, idle, iowait, irq, sirq, stolen, guest, gnice;
@@ -293,13 +301,16 @@ static struct {
   { RS(SYS_DELTA_PROC_CREATED),  RG(SYS_DELTA_PROC_CREATED) },
   { RS(SYS_DELTA_PROC_RUNNING),  RG(SYS_DELTA_PROC_RUNNING) },
 
+ // dummy entry corresponding to PROCPS_STAT_logical_end ...
   { NULL,                        NULL                       }
 };
 
     /* please note,
      * 1st enum MUST be kept in sync with highest TIC type
      * 2nd enum MUST be 1 greater than the highest value of any enum */
+#ifdef ENFORCE_LOGICAL
 enum stat_item PROCPS_STAT_TIC_highest = PROCPS_STAT_TIC_DELTA_GUEST_NICE;
+#endif
 enum stat_item PROCPS_STAT_logical_end = PROCPS_STAT_SYS_DELTA_PROC_RUNNING + 1;
 
 #undef setNAME
@@ -969,7 +980,7 @@ PROCPS_EXPORT struct stat_reaped *procps_stat_reap (
         enum stat_item *items,
         int numitems)
 {
-    int i, rc;
+    int rc;
 
     if (info == NULL || items == NULL)
         return NULL;
@@ -981,12 +992,15 @@ PROCPS_EXPORT struct stat_reaped *procps_stat_reap (
     if (what != STAT_REAP_CPUS_ONLY && what != STAT_REAP_CPUS_AND_NODES)
         return NULL;
 
+#ifdef ENFORCE_LOGICAL
+{   int i;
     // those PROCPS_STAT_SYS_type enum's make sense only to 'select' ...
     for (i = 0; i < numitems; i++) {
         if (items[i] > PROCPS_STAT_TIC_highest)
             return NULL;
     }
-
+}
+#endif
     if ((rc = stacks_reconfig_maybe(&info->cpu_summary, items, numitems)) < 0)
         return NULL;
     if (rc) {
index 2fc5a8633f1abeb10b1bffc4990ea717ebc8595d..7fa3f2c74e67aca944fcf3264863c9b2451fba7d 100644 (file)
@@ -22,8 +22,8 @@
 __BEGIN_DECLS
 
 enum stat_item {
-    PROCPS_STAT_noop,                    //         ( never altered )
-    PROCPS_STAT_extra,                   //         ( reset to zero )
+    PROCPS_STAT_noop,                    //        ( never altered )
+    PROCPS_STAT_extra,                   //        ( reset to zero )
 
     PROCPS_STAT_TIC_ID,                  //   s_int
     PROCPS_STAT_TIC_NUMA_NODE,           //   s_int
@@ -111,14 +111,14 @@ signed long long procps_stat_get (
     struct procps_statinfo *info,
     enum stat_item item);
 
-struct stat_stack *procps_stat_select (
+struct stat_reaped *procps_stat_reap (
     struct procps_statinfo *info,
+    enum stat_reap_type what,
     enum stat_item *items,
     int numitems);
 
-struct stat_reaped *procps_stat_reap (
+struct stat_stack *procps_stat_select (
     struct procps_statinfo *info,
-    enum stat_reap_type what,
     enum stat_item *items,
     int numitems);
 
index 212bdb9df6241cfb74ee0f545dbd5389b58f203b..d0536668f52229015f4e40618e1e12203c6c2333 100644 (file)
 #include <proc/procps-private.h>
 #include <proc/vmstat.h>
 
+
 #define VMSTAT_FILE "/proc/vmstat"
 
+/*
+ *  Perhaps someday we'll all learn what is in these fields. But |
+ *  that might require available linux documentation progressing |
+ *  beyond a state that was acknowledged in the following thread |
+ *
+ *  http://www.spinics.net/lists/linux-man/msg09096.html
+ */
 struct vmstat_data {
     unsigned long allocstall;
     unsigned long balloon_deflate;
@@ -435,6 +443,11 @@ HST_set(DELTA_WORKINGSET_NODERECLAIM,         workingset_nodereclaim)
 HST_set(DELTA_WORKINGSET_REFAULT,             workingset_refault)
 HST_set(DELTA_ZONE_RECLAIM_FAILED,            zone_reclaim_failed)
 
+#undef setDECL
+#undef REG_set
+#undef HST_set
+
+
 // ___ Results 'Get' Support ||||||||||||||||||||||||||||||||||||||||||||||||||
 
 #define getNAME(e) get_results_ ## e
@@ -687,6 +700,10 @@ HST_get(DELTA_WORKINGSET_NODERECLAIM,         workingset_nodereclaim)
 HST_get(DELTA_WORKINGSET_REFAULT,             workingset_refault)
 HST_get(DELTA_ZONE_RECLAIM_FAILED,            zone_reclaim_failed)
 
+#undef getDECL
+#undef REG_get
+#undef HST_get
+
 
 // ___ Controlling Table ||||||||||||||||||||||||||||||||||||||||||||||||||||||
 
@@ -947,6 +964,7 @@ static struct {
   { RS(DELTA_WORKINGSET_REFAULT),            RG(DELTA_WORKINGSET_REFAULT)            },
   { RS(DELTA_ZONE_RECLAIM_FAILED),           RG(DELTA_ZONE_RECLAIM_FAILED)           },
 
+ // dummy entry corresponding to PROCPS_VMSTAT_logical_end ...
   { NULL,                                    NULL                                    }
 };
 
@@ -955,13 +973,7 @@ static struct {
 enum vmstat_item PROCPS_VMSTAT_logical_end = PROCPS_VMSTAT_DELTA_ZONE_RECLAIM_FAILED + 1;
 
 #undef setNAME
-#undef setDECL
-#undef REG_set
-#undef HST_set
 #undef getNAME
-#undef getDECL
-#undef REG_get
-#undef HST_get
 #undef RS
 #undef RG
 
@@ -1074,9 +1086,7 @@ static inline int items_check_failed (
 static int make_hash_failed (
         struct procps_vmstat *info)
 {
- #define mkSTR(a) xySTR(a)
- #define xySTR(z) #z
- #define htVAL(f) e.key  = mkSTR(f); e.data = &info->hist.new. f; \
+ #define htVAL(f) e.key = STRINGIFY(f); e.data = &info->hist.new. f; \
   if (!hsearch_r(e, ENTER, &ep, &info->hashtab)) return -errno;
     ENTRY e, *ep;
     size_t n;
@@ -1205,8 +1215,6 @@ static int make_hash_failed (
     htVAL(zone_reclaim_failed)
 
     return 0;
- #undef mkSTR
- #undef xySTR
  #undef htVAL
 } // end: make_hash_failed
 
@@ -1254,7 +1262,8 @@ static int read_vmstat_failed (
 
     head = buf;
     do {
-        ENTRY e, *ep;
+        static ENTRY e;      // just to keep coverity off our backs (e.data)
+        ENTRY *ep;
 
         tail = strchr(head, ' ');
         if (!tail)
index 0f446df040690f852714f69ddabab6386853232a..9b8ea5e981010c3fe77d7e489ff3d1b3f740ad8f 100644 (file)
@@ -29,8 +29,8 @@
 __BEGIN_DECLS
 
 enum vmstat_item {
-    PROCPS_VMSTAT_noop,                                 //    n/a
-    PROCPS_VMSTAT_extra,                                //    n/a
+    PROCPS_VMSTAT_noop,                                 //       ( never altered )
+    PROCPS_VMSTAT_extra,                                //       ( reset to zero )
 
     PROCPS_VMSTAT_ALLOCSTALL,                           // ul_int
     PROCPS_VMSTAT_BALLOON_DEFLATE,                      // ul_int