]> granicus.if.org Git - procps-ng/commitdiff
library: protect against a future breakage, <pids> ABI
authorJim Warner <james.warner@comcast.net>
Mon, 18 Apr 2016 05:00:00 +0000 (00:00 -0500)
committerCraig Small <csmall@enc.com.au>
Tue, 19 Apr 2016 10:38:18 +0000 (20:38 +1000)
By co-mingling both external/internal identifiers with
actual implementation code, potential future additions
to our API would have been considerably more difficult.

So, this patch will now rely solely on internal/hidden
identifiers serving as fenceposts in validation logic.

And if the following convention is used for new fields
we will maintain that enumerator alphabetic ordering a
a little longer (even though 2 user fields now don't):

. PROCPS_PIDS_XTRA_FOO_A, PROCPS_PIDS_XTRA_FOO_B, etc.

Reference(s):
http://www.freelists.org/post/procps/me-too-newlib,7

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

index 19a142e7f451d99ea3c150d08eaffc7dc8c6d314..69c720c5fb8082e771d596ae18f55c4fc85494c7 100644 (file)
@@ -52,8 +52,9 @@
 
 #define READS_BEGUN (info->read)       // a read is in progress
 
-enum pids_item PROCPS_PIDS_logical_end  = PROCPS_PIDS_noop + 1;
-enum pids_item PROCPS_PIDS_physical_end = PROCPS_PIDS_noop + 2;
+   // next 2 MUST be kept in sync with highest value enum
+enum pids_item PROCPS_PIDS_logical_end  = PROCPS_PIDS_WCHAN_NAME + 1;
+enum pids_item PROCPS_PIDS_physical_end = PROCPS_PIDS_WCHAN_NAME + 2;
 
     // these represent the proc_t fields whose storage cannot be managed
     // optimally if they are ever referenced more than once in any stack
@@ -136,6 +137,8 @@ struct procps_pidsinfo {
     if (I->ref_counts[ref_ ## e2]) R->result.strv = NULL; \
     else { R->result.strv = P-> x; P-> x = NULL; } }
 
+setDECL(noop)         { (void)I; (void)R; (void)P; return; }
+setDECL(extra)        { (void)I; (void)R; (void)P; return; }
 REG_set(ADDR_END_CODE,    ul_int,  end_code)
 REG_set(ADDR_KSTK_EIP,    ul_int,  kstk_eip)
 REG_set(ADDR_KSTK_ESP,    ul_int,  kstk_esp)
@@ -253,10 +256,6 @@ setDECL(VM_USED)      { (void)I; R->result.ul_int = P->vm_swap + P->vm_rss; }
 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)); }
-setDECL(extra)        { (void)I; (void)R; (void)P; return; }
-setDECL(noop)         { (void)I; (void)R; (void)P; return; }
-setDECL(logical_end)  { (void)I; (void)R; (void)P; return; }
-setDECL(physical_end) { (void)I; (void)R; (void)P; return; }
 
 #undef setDECL
 #undef CVT_set
@@ -391,6 +390,8 @@ static struct {
 } Item_table[] = {
 /*    setsfunc               oldflags    freefunc   sortfunc      needhist  refcount
       ---------------------  ----------  ---------  ------------  --------  ------------- */
+    { RS(noop),              0,          NULL,      QS(noop),     0,        -1            }, // user only, never altered
+    { RS(extra),             0,          NULL,      QS(ull_int),  0,        -1            }, // user only, reset to zero
     { RS(ADDR_END_CODE),     f_stat,     NULL,      QS(ul_int),   0,        -1            },
     { RS(ADDR_KSTK_EIP),     f_stat,     NULL,      QS(ul_int),   0,        -1            },
     { RS(ADDR_KSTK_ESP),     f_stat,     NULL,      QS(ul_int),   0,        -1            },
@@ -508,10 +509,8 @@ static struct {
     { RS(VSIZE_PGS),         f_stat,     NULL,      QS(ul_int),   0,        -1            },
     { RS(WCHAN_ADDR),        f_stat,     NULL,      QS(ul_int),   0,        -1            },
     { RS(WCHAN_NAME),        0,          FF(str),   QS(str),      0,        -1            }, // oldflags: tid already free
-    { RS(extra),             0,          NULL,      QS(ull_int),  0,        -1            },
-    { RS(noop),              0,          NULL,      QS(noop),     0,        -1            },
-    { RS(logical_end),       0,          NULL,      QS(noop),     0,        -1            },
-    { RS(physical_end),      0,          NULL,      QS(noop),     0,        -1            }
+   // dummy entry never referenced ( makes future additions 'comma' worry free )...
+    { NULL,                  0,          NULL,      NULL,         0,        -1            }
 };
 
 #undef RS
@@ -784,11 +783,12 @@ static inline void cleanup_stack (
     int i;
 
     for (i = 0; i < depth; i++) {
-        if (p->item < PROCPS_PIDS_noop) {
-            if (Item_table[p->item].freefunc)
-                Item_table[p->item].freefunc(p);
+        if (p->item >= PROCPS_PIDS_logical_end)
+            break;
+        if (Item_table[p->item].freefunc)
+            Item_table[p->item].freefunc(p);
+        if (p->item > PROCPS_PIDS_noop)
             p->result.ull_int = 0;
-        }
         ++p;
     }
 } // end: cleanup_stack
@@ -869,7 +869,7 @@ static inline int items_check_failed (
         // a pids_item is currently unsigned, but we'll protect our future
         if (items[i] < 0)
             return -1;
-        if (items[i] > PROCPS_PIDS_noop) {
+        if (items[i] >= PROCPS_PIDS_logical_end) {
             return -1;
         }
     }
@@ -1023,7 +1023,7 @@ static void validate_stacks (
  * alloc_stacks():
  *
  * Allocate and initialize one or more stacks each of which is anchored in an
- * associated pids_stack structure (which may include extra user space).
+ * associated pids_stack structure.
  *
  * All such stacks will will have their result structures properly primed with
  * 'items', while the result itself will be zeroed.
@@ -1438,7 +1438,7 @@ PROCPS_EXPORT struct pids_stack **procps_pids_sort (
     if (info == NULL || stacks == NULL)
         return NULL;
     // a pids_item is currently unsigned, but we'll protect our future
-    if (sort < 0  || sort > PROCPS_PIDS_noop)
+    if (sort < 0  || sort >= PROCPS_PIDS_logical_end)
         return NULL;
     if (order != PROCPS_SORT_ASCEND && order != PROCPS_SORT_DESCEND)
         return NULL;
@@ -1453,7 +1453,7 @@ PROCPS_EXPORT struct pids_stack **procps_pids_sort (
         ++offset;
         if (offset >= info->curitems)
             return NULL;
-        if (p->item > PROCPS_PIDS_noop)
+        if (p->item >= PROCPS_PIDS_logical_end)
             return NULL;
         ++p;
     }
index bfc977c4a8c312fdc3cd652b0bc5de0a3494cce8..2d2abb49c49db634d6da76277622afec6b725bab 100644 (file)
@@ -27,6 +27,8 @@
 __BEGIN_DECLS
 
 enum pids_item {
+    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
@@ -144,8 +146,6 @@ enum pids_item {
     PROCPS_PIDS_VSIZE_PGS,             // ul_int
     PROCPS_PIDS_WCHAN_ADDR,            // ul_int
     PROCPS_PIDS_WCHAN_NAME,            // str
-    PROCPS_PIDS_extra,                 //         ( reset to zero )
-    PROCPS_PIDS_noop                   //         ( never altered )
 };
 
 enum pids_fill_type {