library: tweak 'other' user/group names for efficiency

This commit just ensures that the relatively expensive
ID to name conversions aren't performed unless they're
explicitly requested. It also internalizes those flags
that required the PROC_FILLSTATUS flag to also be set.

[ requiring a caller, in our case pids.c, to provide ]
[ two flags when a single field was the objective is ]
[ wrong & represents a future potential toe-stubber. ]

[ moreover, what's worse is that those two flags are ]
[ seemingly unrelated. but, without both, a SEGV can ]
[ can be expected when a result.str pointer is NULL. ]

[ by contrast, in the master branch those fields are ]
[ arrays which, when set to zeroes, produce an empty ]
[ string. So, there is no abend (but no name either) ]
[ when one of those two required flags were omitted. ]

[ and worth noting, in that branch it's not just one ]
[ caller required to observe a two flag requirement. ]

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

diff --git a/proc/pids.c b/proc/pids.c
index 3ff0605410605e50d348184d8da58cb6fb847e8a..50212b7302e36367128e5032e6f46790f45006ef 100644 (file)
--- a/proc/pids.c
+++ b/proc/pids.c
@@ -356,10 +356,10 @@ srtDECL(noop) {
 #define v_arg      PROC_FILLARG
 #define v_cgroup   PROC_FILLCGROUP
 #define v_env      PROC_FILLENV
-   // remaining are compound flags
-#define x_ogroup   PROC_FILLSTATUS  | PROC_FILLGRP
-#define x_ouser    PROC_FILLSTATUS  | PROC_FILLUSR
-#define x_supgrp   PROC_FILLSTATUS  | PROC_FILLSUPGRP
+   // these next three will also force PROC_FILLSTATUS
+#define x_ogroup   PROC_FILL_OGROUPS
+#define x_ouser    PROC_FILL_OUSERS
+#define x_supgrp   PROC_FILL_SUPGRP
 
 typedef void (*SET_t)(struct pids_info *, struct pids_result *, proc_t *);
 typedef void (*FRE_t)(struct pids_result *);

diff --git a/proc/readproc.c b/proc/readproc.c
index 0819629bbc735d3813de592be409128871c4ef3c..bc92fa7b8eed8b744be03c32d7014f678641341e 100644 (file)
--- a/proc/readproc.c
+++ b/proc/readproc.c
@@ -1051,8 +1051,18 @@ static proc_t* simple_readproc(PROCTAB *restrict const PT, proc_t *restrict cons
     if (flags & PROC_FILLSTATUS) {              // read /proc/#/status
         if (file2str(path, "status", &ub) != -1){
             rc += status2proc(ub.buf, p, 1);
-            if (flags & PROC_FILLSUPGRP)
+            if (flags & (PROC_FILL_SUPGRP & ~PROC_FILLSTATUS))
                 rc += supgrps_from_supgids(p);
+            if (flags & (PROC_FILL_OUSERS & ~PROC_FILLSTATUS)) {
+                p->ruser = pwcache_get_user(p->ruid);
+                p->suser = pwcache_get_user(p->suid);
+                p->fuser = pwcache_get_user(p->fuid);
+            }
+            if (flags & (PROC_FILL_OGROUPS & ~PROC_FILLSTATUS)) {
+                p->rgroup = pwcache_get_group(p->rgid);
+                p->sgroup = pwcache_get_group(p->sgid);
+                p->fgroup = pwcache_get_group(p->fgid);
+            }
         }
     }
 
@@ -1063,25 +1073,10 @@ static proc_t* simple_readproc(PROCTAB *restrict const PT, proc_t *restrict cons
 
     /* some number->text resolving which is time consuming */
     /* ( names are cached, so memcpy to arrays was silly ) */
-    if (flags & PROC_FILLUSR){
+    if (flags & PROC_FILLUSR)
         p->euser = pwcache_get_user(p->euid);
-        if(flags & PROC_FILLSTATUS) {
-            p->ruser = pwcache_get_user(p->ruid);
-            p->suser = pwcache_get_user(p->suid);
-            p->fuser = pwcache_get_user(p->fuid);
-        }
-    }
-
-    /* some number->text resolving which is time consuming */
-    /* ( names are cached, so memcpy to arrays was silly ) */
-    if (flags & PROC_FILLGRP){
+    if (flags & PROC_FILLGRP)
         p->egroup = pwcache_get_group(p->egid);
-        if(flags & PROC_FILLSTATUS) {
-            p->rgroup = pwcache_get_group(p->rgid);
-            p->sgroup = pwcache_get_group(p->sgid);
-            p->fgroup = pwcache_get_group(p->fgid);
-        }
-    }
 
     if (flags & PROC_FILLENV)                   // read /proc/#/environ
         if (!(p->environ_v = file2strvec(path, "environ")))
@@ -1165,32 +1160,27 @@ static proc_t* simple_readtask(PROCTAB *restrict const PT, proc_t *restrict cons
     if (flags & PROC_FILLSTATUS) {                      // read /proc/#/task/#/status
         if (file2str(path, "status", &ub) != -1) {
             rc += status2proc(ub.buf, t, 0);
-            if (flags & PROC_FILLSUPGRP)
+            if (flags & (PROC_FILL_SUPGRP & ~PROC_FILLSTATUS))
                 rc += supgrps_from_supgids(t);
+            if (flags & (PROC_FILL_OUSERS & ~PROC_FILLSTATUS)) {
+                t->ruser = pwcache_get_user(t->ruid);
+                t->suser = pwcache_get_user(t->suid);
+                t->fuser = pwcache_get_user(t->fuid);
+            }
+            if (flags & (PROC_FILL_OGROUPS & ~PROC_FILLSTATUS)) {
+                t->rgroup = pwcache_get_group(t->rgid);
+                t->sgroup = pwcache_get_group(t->sgid);
+                t->fgroup = pwcache_get_group(t->fgid);
+            }
         }
     }
 
     /* some number->text resolving which is time consuming */
     /* ( names are cached, so memcpy to arrays was silly ) */
-    if (flags & PROC_FILLUSR){
+    if (flags & PROC_FILLUSR)
         t->euser = pwcache_get_user(t->euid);
-        if(flags & PROC_FILLSTATUS) {
-            t->ruser = pwcache_get_user(t->ruid);
-            t->suser = pwcache_get_user(t->suid);
-            t->fuser = pwcache_get_user(t->fuid);
-        }
-    }
-
-    /* some number->text resolving which is time consuming */
-    /* ( names are cached, so memcpy to arrays was silly ) */
-    if (flags & PROC_FILLGRP){
+    if (flags & PROC_FILLGRP)
         t->egroup = pwcache_get_group(t->egid);
-        if(flags & PROC_FILLSTATUS) {
-            t->rgroup = pwcache_get_group(t->rgid);
-            t->sgroup = pwcache_get_group(t->sgid);
-            t->fgroup = pwcache_get_group(t->fgid);
-        }
-    }
 
 #ifdef FALSE_THREADS
     if (!IS_THREAD(t)) {

diff --git a/proc/readproc.h b/proc/readproc.h
index 17cf348da45e3a5a1618e4d722969670460e2cbf..25be6d65e44cf2763173616992640be992878225 100644 (file)
--- a/proc/readproc.h
+++ b/proc/readproc.h
@@ -207,7 +207,6 @@ typedef struct PROCTAB {
 #define PROC_FILLSTATUS      0x0020 // read status
 #define PROC_FILLSTAT        0x0040 // read stat
 #define PROC_FILLCGROUP      0x0200 // alloc and fill in `cgroup` vectors
-#define PROC_FILLSUPGRP      0x0400 // resolve supplementary group id -> group name
 #define PROC_FILLOOM         0x0800 // fill in proc_t oom_score and oom_adj
 #define PROC_FILLNS          0x8000 // fill in proc_t namespace information
 #define PROC_FILLSYSTEMD    0x80000 // fill in proc_t systemd information
@@ -224,6 +223,11 @@ typedef struct PROCTAB {
 #define PROC_EDITCMDLCVT    0x20000 // edit `cmdline' as regular string
 #define PROC_EDITENVRCVT    0x40000 // edit `environ' as regular string
 
+// these three also require the PROC_FILLSTATUS flage
+#define PROC_FILL_OUSERS   ( 0x0080 | PROC_FILLSTATUS ) // obtain other user names
+#define PROC_FILL_OGROUPS  ( 0x0100 | PROC_FILLSTATUS ) // obtain other group names
+#define PROC_FILL_SUPGRP   ( 0x0400 | PROC_FILLSTATUS ) // obtain supplementary group names
+
 // it helps to give app code a few spare bits
 #define PROC_SPARE_1     0x01000000
 #define PROC_SPARE_2     0x02000000