library: refactor exposed pointers management, 3rd gen

This commit brings all of those 'fetch' type functions
(supporting some form of 'reap') into closer alignment
with one another. The biggest impact is to be found in
the <stat> module, which now provides for the separate
copy of stack pointers which will be exposed to users.

The reason such a copy was not employed initially with
<stat>, unlike those for <pids> and <slabinfo>, is due
to the fact that such stacks were never sortable. Thus
the original raw consolidated extent pointers wouldn't
have been disturbed. But that meant no NULL delimiter.

So with this commit, all reap/fetch operations now use
pointer copies when returning results to callers. And,
all such arrays are now NULL delimited meaning callers
can choose their own access fencepost: totals or NULL.

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

diff --git a/proc/pids.c b/proc/pids.c
index 23c97af861e8ae0b9976507909364d72644fff6d..0e36ed144b71dfaff03be9a392956e8c9bfcd99d 100644 (file)
--- a/proc/pids.c
+++ b/proc/pids.c
@@ -1090,14 +1090,16 @@ 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;
+             are exposed to a user (um, not that we don't trust 'em or anything). |
+             plus, we can NULL delimit these ptrs which we couldn't do otherwise. | */
+    if (n_saved < n_inuse + 1) {
+        n_saved = n_inuse + 1;
         if (!(info->fetch.results.stacks = realloc(info->fetch.results.stacks, sizeof(void *) * n_saved)))
-            return -1;
+            return -ENOMEM;
     }
     memcpy(info->fetch.results.stacks, info->fetch.anchor, sizeof(void *) * n_inuse);
     info->fetch.results.stacks[n_inuse] = NULL;
+
     return n_inuse;     // callers beware, this might be zero !
  #undef n_alloc
  #undef n_inuse

diff --git a/proc/slabinfo.c b/proc/slabinfo.c
index f5dda453b1fa36863efe228df150a0b496f36682..7633ad792962bac53590d867089a858ce426d95b 100644 (file)
--- a/proc/slabinfo.c
+++ b/proc/slabinfo.c
@@ -45,6 +45,8 @@
 #define SLABINFO_LINE_LEN    2048
 #define SLAB_INFO_NAME_LEN   128
 
+#define STACKS_INCR          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, |
@@ -682,7 +684,6 @@ static struct stacks_extent *stacks_alloc (
 static int stacks_fetch (
         struct procps_slabinfo *info)
 {
- #define memINCR  64
  #define n_alloc  info->fetch.n_alloc
  #define n_inuse  info->fetch.n_inuse
  #define n_saved  info->fetch.n_alloc_save
@@ -690,9 +691,9 @@ static int stacks_fetch (
 
     // initialize stuff -----------------------------------
     if (!info->fetch.anchor) {
-        if (!(info->fetch.anchor = calloc(sizeof(void *), memINCR)))
+        if (!(info->fetch.anchor = calloc(sizeof(void *), STACKS_INCR)))
             return -ENOMEM;
-        n_alloc = memINCR;
+        n_alloc = STACKS_INCR;
     }
     if (!info->fetch_ext.extents) {
         if (!(ext = stacks_alloc(&info->fetch_ext, n_alloc)))
@@ -707,11 +708,11 @@ static int stacks_fetch (
     n_inuse = 0;
     while (n_inuse < info->nodes_used) {
         if (!(n_inuse < n_alloc)) {
-            n_alloc += memINCR;
+            n_alloc += STACKS_INCR;
             if ((!(info->fetch.anchor = realloc(info->fetch.anchor, sizeof(void *) * n_alloc)))
-            || (!(ext = stacks_alloc(&info->fetch_ext, memINCR))))
+            || (!(ext = stacks_alloc(&info->fetch_ext, STACKS_INCR))))
                 return -1;
-            memcpy(info->fetch.anchor + n_inuse, ext->stacks, sizeof(void *) * memINCR);
+            memcpy(info->fetch.anchor + n_inuse, ext->stacks, sizeof(void *) * STACKS_INCR);
         }
         assign_results(info->fetch.anchor[n_inuse], &info->hist, &info->nodes[n_inuse]);
         ++n_inuse;
@@ -720,18 +721,18 @@ 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;
+             are exposed to a user (um, not that we don't trust 'em or anything). |
+             plus, we can NULL delimit these ptrs which we couldn't do otherwise. | */
+    if (n_saved < n_inuse + 1) {
+        n_saved = n_inuse + 1;
         if (!(info->fetch.results.stacks = realloc(info->fetch.results.stacks, sizeof(void *) * n_saved)))
-            return -1;
+            return -ENOMEM;
     }
     memcpy(info->fetch.results.stacks, info->fetch.anchor, sizeof(void *) * n_inuse);
     info->fetch.results.stacks[n_inuse] = NULL;
     info->fetch.results.total = n_inuse;
 
     return n_inuse;
- #undef memINCR
  #undef n_alloc
  #undef n_inuse
  #undef n_saved

diff --git a/proc/stat.c b/proc/stat.c
index a1a4a2dffb1f6b49e1f0f222093781a30357a6ec..f0437fa6d0edb95911331fdcb978d3df1695f349 100644 (file)
--- a/proc/stat.c
+++ b/proc/stat.c
@@ -105,8 +105,9 @@ struct reap_support {
     int total;                         // independently obtained # of cpus/nodes
     struct ext_support fetch;          // extents plus items details
     struct tic_support hist;           // cpu and node jiffies management
-    int n_anchor_alloc;                // last known anchor pointers allocation
+    int n_alloc;                       // last known anchor pointers allocation
     struct stat_stack **anchor;        // reapable stacks (consolidated extents)
+    int n_alloc_save;                  // last known results.stacks allocation
     struct stat_reap result;           // summary + stacks returned to caller
 };
 
@@ -649,6 +650,9 @@ static int stacks_fetch_tics (
         struct procps_statinfo *info,
         struct reap_support *this)
 {
+ #define n_alloc  this->n_alloc
+ #define n_inuse  this->hist.n_inuse
+ #define n_saved  this->n_alloc_save
     struct stacks_extent *ext;
     int i;
 
@@ -659,21 +663,21 @@ static int stacks_fetch_tics (
     if (!this->anchor) {
         if (!(this->anchor = calloc(sizeof(void *), STACKS_INCR)))
             return -ENOMEM;
-        this->n_anchor_alloc = STACKS_INCR;
+        n_alloc = STACKS_INCR;
     }
     if (!this->fetch.extents) {
-        if (!(ext = stacks_alloc(&this->fetch, this->n_anchor_alloc)))
+        if (!(ext = stacks_alloc(&this->fetch, n_alloc)))
             return -ENOMEM;
-        memcpy(this->anchor, ext->stacks, sizeof(void *) * this->n_anchor_alloc);
+        memcpy(this->anchor, ext->stacks, sizeof(void *) * n_alloc);
     }
     if (this->fetch.dirty_stacks)
         cleanup_stacks_all(&this->fetch);
 
     // iterate stuff --------------------------------------
-    for (i = 0; i < this->hist.n_inuse; i++) {
-        if (!(i < this->n_anchor_alloc)) {
-            this->n_anchor_alloc += STACKS_INCR;
-            if ((!(this->anchor = realloc(this->anchor, sizeof(void *) * this->n_anchor_alloc)))
+    for (i = 0; i < n_inuse; i++) {
+        if (!(i < n_alloc)) {
+            n_alloc += STACKS_INCR;
+            if ((!(this->anchor = realloc(this->anchor, sizeof(void *) * n_alloc)))
             || (!(ext = stacks_alloc(&this->fetch, STACKS_INCR)))) {
                 return -ENOMEM;
             }
@@ -683,12 +687,25 @@ static int stacks_fetch_tics (
     }
 
     // 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 a user (um, not that we don't trust 'em or anything). |
+             plus, we can NULL delimit these ptrs which we couldn't do otherwise. | */
+    if (n_saved < i + 1) {
+        n_saved = i + 1;
+        if (!(this->result.stacks = realloc(this->result.stacks, sizeof(void *) * n_saved)))
+            return -ENOMEM;
+    }
+    memcpy(this->result.stacks, this->anchor, sizeof(void *) * i);
+    this->result.stacks[i] = NULL;
     this->result.total = i;
-    this->result.stacks = this->anchor;
     this->fetch.dirty_stacks = 1;
 
     // callers beware, this might be zero (maybe no libnuma.so) ...
     return this->result.total;
+ #undef n_alloc
+ #undef n_inuse
+ #undef n_saved
 } // end: stacks_fetch_tics
 
 
@@ -766,6 +783,7 @@ PROCPS_EXPORT int procps_stat_new (
 
     p->refcount = 1;
     p->stat_fd = -1;
+
     p->results.cpus = &p->cpus.result;
     p->results.nodes = &p->nodes.result;
     p->cpus.total = procps_cpu_count();
@@ -824,6 +842,8 @@ PROCPS_EXPORT int procps_stat_unref (
     if ((*info)->refcount == 0) {
         if ((*info)->cpus.anchor)
             free((*info)->cpus.anchor);
+        if ((*info)->cpus.result.stacks)
+            free((*info)->cpus.result.stacks);
         if ((*info)->cpus.hist.tics)
             free((*info)->cpus.hist.tics);
         if ((*info)->cpus.fetch.extents)
@@ -831,6 +851,8 @@ PROCPS_EXPORT int procps_stat_unref (
 
         if ((*info)->nodes.anchor)
             free((*info)->nodes.anchor);
+        if ((*info)->nodes.result.stacks)
+            free((*info)->nodes.result.stacks);
         if ((*info)->nodes.hist.tics)
             free((*info)->nodes.hist.tics);
         if ((*info)->nodes.fetch.extents)
@@ -911,11 +933,6 @@ PROCPS_EXPORT struct stat_reaped *procps_stat_reap (
 
     if (info == NULL || items == NULL)
         return NULL;
-
-    info->results.summary = NULL;
-    info->cpus.result.total = info->nodes.result.total = 0;
-    info->cpus.result.stacks = info->nodes.result.stacks = NULL;
-
     if (what != STAT_REAP_CPUS_ONLY && what != STAT_REAP_CPUS_AND_NODES)
         return NULL;
 
@@ -940,6 +957,15 @@ PROCPS_EXPORT struct stat_reaped *procps_stat_reap (
         return NULL;
     info->results.summary = update_single_stack(info, &info->cpu_summary);
 
+    /* unlike the other 'reap' functions, <stat> provides for two separate
+       stacks pointer arrays exposed to callers. Thus, to keep our promise
+       of NULL delimit we must ensure a minimal array for the optional one */
+    if (!info->nodes.result.stacks
+    && (!(info->nodes.result.stacks = malloc(sizeof(void *)))))
+        return NULL;
+    info->nodes.result.total = 0;
+    info->nodes.result.stacks[0] = NULL;
+
     switch (what) {
         case STAT_REAP_CPUS_ONLY:
             if (!stacks_fetch_tics(info, &info->cpus))