Simplify #1109

alejandro-colomar · 2024-11-04T16:55:14Z

Revisions:

v1b

Reformat function declarator to show the correct function name in git-diff(1) hunk contexts.

$ git range-diff master gh/S S 
 1:  a84dfc09 =  1:  a84dfc09 src/: Transform do-while into while
 2:  215be064 =  2:  215be064 lib/gshadow.c: build_list(): Fix type of parameter
 3:  a8d604d9 =  3:  a8d604d9 lib/gshadow.c: build_list(): Remove unused variable
 4:  8ee64c91 =  4:  8ee64c91 lib/gshadow.c: build_list(): Improve variable and parameter names
 5:  3992b452 !  5:  c765251f lib/gshadow.c: sgetsgent(): Remove superfluous condition
    @@ Commit message
     
      ## lib/gshadow.c ##
     @@ lib/gshadow.c: void endsgent (void)
    +   shadow = NULL;
    + }
    + 
    +-/*@observer@*//*@null@*/struct sgrp *sgetsgent (const char *string)
    ++/*@observer@*//*@null@*/struct sgrp *
    ++sgetsgent(const char *string)
    + {
    +   static char *sgrbuf = NULL;
    +   static size_t sgrbuflen = 0;
    +@@ lib/gshadow.c: void endsgent (void)
      
        sgroup.sg_name = fields[0];
        sgroup.sg_passwd = fields[1];
 6:  1f019a3b !  6:  7ae4212d lib/gshadow.c: Move list cleanup to within build_list()
    @@ lib/gshadow.c: static /*@null@*/char **
      
        while (s != NULL && *s != '\0') {
                l = XREALLOC(*lp, n + 1, char *);
    -@@ lib/gshadow.c: void endsgent (void)
    +@@ lib/gshadow.c: sgetsgent(const char *string)
        sgroup.sg_name = fields[0];
        sgroup.sg_passwd = fields[1];
      
 7:  1540998a =  7:  cb7c2e2e lib/gshadow.c: build_list(): Minimize use of pointer parameters
 8:  a76af831 =  8:  a996c741 lib/gshadow.c: build_list(): Compact ++ into previous statement
 9:  8c852922 !  9:  554e5fe6 lib/gshadow.c: Remove dead code
    @@ lib/gshadow.c: build_list(char *s, char ***lp, size_t *np)
      
        return l;
      }
    -@@ lib/gshadow.c: void endsgent (void)
    +@@ lib/gshadow.c: sgetsgent(const char *string)
        sgroup.sg_name = fields[0];
        sgroup.sg_passwd = fields[1];

v2

Use NULL, not 0.

$ git range-diff master gh/S S
 1:  a84dfc09 =  1:  a84dfc09 src/: Transform do-while into while
 2:  215be064 =  2:  215be064 lib/gshadow.c: build_list(): Fix type of parameter
 3:  a8d604d9 =  3:  a8d604d9 lib/gshadow.c: build_list(): Remove unused variable
 4:  8ee64c91 =  4:  8ee64c91 lib/gshadow.c: build_list(): Improve variable and parameter names
 5:  c765251f =  5:  c765251f lib/gshadow.c: sgetsgent(): Remove superfluous condition
 6:  7ae4212d =  6:  7ae4212d lib/gshadow.c: Move list cleanup to within build_list()
 7:  cb7c2e2e =  7:  cb7c2e2e lib/gshadow.c: build_list(): Minimize use of pointer parameters
 8:  a996c741 =  8:  a996c741 lib/gshadow.c: build_list(): Compact ++ into previous statement
 9:  554e5fe6 =  9:  554e5fe6 lib/gshadow.c: Remove dead code
 -:  -------- > 10:  ac58bc28 lib/gshadow.c: sgetsgent(): Be consistent using NULL

v3

Reorganize the commits
free(3) before calling build_list()

$ git range-diff master gh/S S 
 1:  a84dfc09 =  1:  a84dfc09 src/: Transform do-while into while
 2:  215be064 =  2:  215be064 lib/gshadow.c: build_list(): Fix type of parameter
 3:  a8d604d9 =  3:  a8d604d9 lib/gshadow.c: build_list(): Remove unused variable
 4:  8ee64c91 =  4:  8ee64c91 lib/gshadow.c: build_list(): Improve variable and parameter names
 -:  -------- >  5:  53f8bfb0 lib/gshadow.c: build_list(): Remove dead assignment
 5:  c765251f =  6:  cdbb5ef0 lib/gshadow.c: sgetsgent(): Remove superfluous condition
 6:  7ae4212d !  7:  d47ca91d lib/gshadow.c: Move list cleanup to within build_list()
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>
     
      ## Commit message ##
    -    lib/gshadow.c: Move list cleanup to within build_list()
    +    lib/gshadow.c: Move zeroing to within build_list()
     
         This makes build_list() less dependent on the context.
         It starts from clean, whatever was the state before the call.
         I was having a hard time understanding the reallocation,
    -    until I saw that we were
    -    freeing and NULLing everything right before the call.
    +    until I saw that we were zeroing everything right before the call.
     
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
    @@ lib/gshadow.c
     @@ lib/gshadow.c: static /*@null@*/char **
      build_list(char *s, char ***lp, size_t *np)
      {
    -   char    **l = *lp;
    +   char    **l;
     -  size_t  n = *np;
     +  size_t  n;
     +
    -+  n = 0;
    -+  free(*lp);
     +  *lp = NULL;
    ++  n = 0;
      
        while (s != NULL && *s != '\0') {
                l = XREALLOC(*lp, n + 1, char *);
    @@ lib/gshadow.c: sgetsgent(const char *string)
     -  nmembers = 0;
     -  free (members);
     -  members = NULL;
    --
    ++  free(admins);
    ++  free(members);
    + 
        sgroup.sg_adm = build_list (fields[2], &admins, &nadmins);
        sgroup.sg_mem = build_list (fields[3], &members, &nmembers);
    - 
 7:  cb7c2e2e !  8:  d2ae59c9 lib/gshadow.c: build_list(): Minimize use of pointer parameters
    @@ Commit message
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## lib/gshadow.c ##
    -@@ lib/gshadow.c: static struct sgrp sgroup;
    - static /*@null@*/char **
    - build_list(char *s, char ***lp, size_t *np)
    - {
    --  char    **l = *lp;
    -+  char    **l;
    +@@ lib/gshadow.c: build_list(char *s, char ***lp, size_t *np)
    +   char    **l;
        size_t  n;
      
    --  n = 0;
    -   free(*lp);
     -  *lp = NULL;
    -+
     +  l = NULL;
    -+  n = 0;
    +   n = 0;
      
        while (s != NULL && *s != '\0') {
     -          l = XREALLOC(*lp, n + 1, char *);
 8:  a996c741 =  9:  6ea8e5f0 lib/gshadow.c: build_list(): Compact ++ into previous statement
 9:  554e5fe6 ! 10:  184de49c lib/gshadow.c: Remove dead code
    @@ lib/gshadow.c: build_list(char *s, char ***lp, size_t *np)
        return l;
      }
     @@ lib/gshadow.c: sgetsgent(const char *string)
    -   sgroup.sg_name = fields[0];
    -   sgroup.sg_passwd = fields[1];
    +   free(admins);
    +   free(members);
      
     -  sgroup.sg_adm = build_list (fields[2], &admins, &nadmins);
     -  sgroup.sg_mem = build_list (fields[3], &members, &nmembers);
10:  ac58bc28 = 11:  7622cc1c lib/gshadow.c: sgetsgent(): Be consistent using NULL

$ git diff ac58bc28..7622cc1c
diff --git a/lib/gshadow.c b/lib/gshadow.c
index 1a7dbdc4..5caf4f16 100644
--- a/lib/gshadow.c
+++ b/lib/gshadow.c
@@ -40,8 +40,6 @@ build_list(char *s, char ***lp)
        char    **l;
        size_t  n;
 
-       free(*lp);
-
        l = NULL;
        n = 0;
 
@@ -118,6 +116,9 @@ sgetsgent(const char *string)
        sgroup.sg_name = fields[0];
        sgroup.sg_passwd = fields[1];
 
+       free(admins);
+       free(members);
+
        sgroup.sg_adm = build_list(fields[2], &admins);
        sgroup.sg_mem = build_list(fields[3], &members);

v4

Do not be a 3-star programmer! :-)

$ git range-diff master gh/S S
 1:  a84dfc09 =  1:  a84dfc09 src/: Transform do-while into while
 2:  215be064 =  2:  215be064 lib/gshadow.c: build_list(): Fix type of parameter
 3:  a8d604d9 =  3:  a8d604d9 lib/gshadow.c: build_list(): Remove unused variable
 4:  8ee64c91 =  4:  8ee64c91 lib/gshadow.c: build_list(): Improve variable and parameter names
 5:  53f8bfb0 =  5:  53f8bfb0 lib/gshadow.c: build_list(): Remove dead assignment
 6:  cdbb5ef0 =  6:  cdbb5ef0 lib/gshadow.c: sgetsgent(): Remove superfluous condition
 7:  d47ca91d =  7:  d47ca91d lib/gshadow.c: Move zeroing to within build_list()
 8:  d2ae59c9 =  8:  d2ae59c9 lib/gshadow.c: build_list(): Minimize use of pointer parameters
 9:  6ea8e5f0 =  9:  6ea8e5f0 lib/gshadow.c: build_list(): Compact ++ into previous statement
10:  184de49c = 10:  184de49c lib/gshadow.c: Remove dead code
11:  7622cc1c = 11:  7622cc1c lib/gshadow.c: sgetsgent(): Be consistent using NULL
 -:  -------- > 12:  1abba41f lib/gshadow.c: build_list(): Remove second parameter

v5

Remove redundant variables
Move whitespace change to new commit

$ git range-diff master gh/S S 
 1:  a84dfc09 =  1:  a84dfc09 src/: Transform do-while into while
 2:  215be064 =  2:  215be064 lib/gshadow.c: build_list(): Fix type of parameter
 3:  a8d604d9 =  3:  a8d604d9 lib/gshadow.c: build_list(): Remove unused variable
 4:  8ee64c91 =  4:  8ee64c91 lib/gshadow.c: build_list(): Improve variable and parameter names
 5:  53f8bfb0 =  5:  53f8bfb0 lib/gshadow.c: build_list(): Remove dead assignment
 6:  cdbb5ef0 =  6:  cdbb5ef0 lib/gshadow.c: sgetsgent(): Remove superfluous condition
 7:  d47ca91d !  7:  4dc64ba1 lib/gshadow.c: Move zeroing to within build_list()
    @@ lib/gshadow.c: sgetsgent(const char *string)
        sgroup.sg_passwd = fields[1];
      
     -  nadmins = 0;
    --  free (admins);
    +   free (admins);
     -  admins = NULL;
     -  nmembers = 0;
    --  free (members);
    +   free (members);
     -  members = NULL;
    -+  free(admins);
    -+  free(members);
      
        sgroup.sg_adm = build_list (fields[2], &admins, &nadmins);
        sgroup.sg_mem = build_list (fields[3], &members, &nmembers);
 8:  d2ae59c9 =  8:  91a20ba9 lib/gshadow.c: build_list(): Minimize use of pointer parameters
 9:  6ea8e5f0 =  9:  1c82588c lib/gshadow.c: build_list(): Compact ++ into previous statement
10:  184de49c ! 10:  cd30da3d lib/gshadow.c: Remove dead code
    @@ lib/gshadow.c: build_list(char *s, char ***lp, size_t *np)
        return l;
      }
     @@ lib/gshadow.c: sgetsgent(const char *string)
    -   free(admins);
    -   free(members);
    +   free (admins);
    +   free (members);
      
     -  sgroup.sg_adm = build_list (fields[2], &admins, &nadmins);
     -  sgroup.sg_mem = build_list (fields[3], &members, &nmembers);
11:  7622cc1c = 11:  a1db95c8 lib/gshadow.c: sgetsgent(): Be consistent using NULL
12:  1abba41f ! 12:  8d686837 lib/gshadow.c: build_list(): Remove second parameter
    @@ lib/gshadow.c: build_list(char *s, char ***lp)
      }
      
     @@ lib/gshadow.c: sgetsgent(const char *string)
    -   free(admins);
    -   free(members);
    +   free (admins);
    +   free (members);
      
     -  sgroup.sg_adm = build_list(fields[2], &admins);
     -  sgroup.sg_mem = build_list(fields[3], &members);
 -:  -------- > 13:  cb12f334 lib/gshadow.c: Remove redundant variables

$ git diff 1abba41f..cb12f334
diff --git a/lib/gshadow.c b/lib/gshadow.c
index 7de52aaf..0a912b79 100644
--- a/lib/gshadow.c
+++ b/lib/gshadow.c
@@ -27,9 +27,7 @@
 
 
 static /*@null@*/FILE *shadow;
-static /*@null@*//*@only@*/char **members = NULL;
-static /*@null@*//*@only@*/char **admins = NULL;
-static struct sgrp sgroup;
+static struct sgrp  sgroup = {};
 
 #define        FIELDS  4
 
@@ -114,11 +112,11 @@ sgetsgent(const char *string)
        sgroup.sg_name = fields[0];
        sgroup.sg_passwd = fields[1];
 
-       free(admins);
-       free(members);
+       free(sgroup.sg_adm);
+       free(sgroup.sg_mem);
 
-       sgroup.sg_adm = admins  = build_list(fields[2]);
-       sgroup.sg_mem = members = build_list(fields[3]);
+       sgroup.sg_adm = build_list(fields[2]);
+       sgroup.sg_mem = build_list(fields[3]);
 
        return &sgroup;
 }

v6

Simplify even further, by adding and calling strchrcnt(), which allows us to transform the XREALLOC() calls into a single XMALLOC() call.

$ git range-diff master gh/S S 
 1:  a84dfc09 =  1:  a84dfc09 src/: Transform do-while into while
 2:  215be064 =  2:  215be064 lib/gshadow.c: build_list(): Fix type of parameter
 3:  a8d604d9 =  3:  a8d604d9 lib/gshadow.c: build_list(): Remove unused variable
 4:  8ee64c91 =  4:  8ee64c91 lib/gshadow.c: build_list(): Improve variable and parameter names
 5:  53f8bfb0 =  5:  53f8bfb0 lib/gshadow.c: build_list(): Remove dead assignment
 6:  cdbb5ef0 =  6:  cdbb5ef0 lib/gshadow.c: sgetsgent(): Remove superfluous condition
 7:  4dc64ba1 =  7:  4dc64ba1 lib/gshadow.c: Move zeroing to within build_list()
 8:  91a20ba9 =  8:  91a20ba9 lib/gshadow.c: build_list(): Minimize use of pointer parameters
 9:  1c82588c =  9:  1c82588c lib/gshadow.c: build_list(): Compact ++ into previous statement
10:  cd30da3d = 10:  cd30da3d lib/gshadow.c: Remove dead code
11:  a1db95c8 = 11:  a1db95c8 lib/gshadow.c: sgetsgent(): Be consistent using NULL
12:  8d686837 = 12:  8d686837 lib/gshadow.c: build_list(): Remove second parameter
13:  cb12f334 = 13:  cb12f334 lib/gshadow.c: Remove redundant variables
 -:  -------- > 14:  ea50249d lib/string/strchr/: strchrcnt(): Add function
 -:  -------- > 15:  4d9d6c92 lib/gshadow.c: build_list(): Allocate at once
 -:  -------- > 16:  d5218548 lib/gshadow.c: build_list(): Transform while loop into for loop

v7

Don't exit(3) on error in library code. Use MALLOC() instead of XMALLOC().

$ git range-diff master gh/S S 
 1:  a84dfc09 =  1:  a84dfc09 src/: Transform do-while into while
 2:  215be064 =  2:  215be064 lib/gshadow.c: build_list(): Fix type of parameter
 3:  a8d604d9 =  3:  a8d604d9 lib/gshadow.c: build_list(): Remove unused variable
 4:  8ee64c91 =  4:  8ee64c91 lib/gshadow.c: build_list(): Improve variable and parameter names
 5:  53f8bfb0 =  5:  53f8bfb0 lib/gshadow.c: build_list(): Remove dead assignment
 6:  cdbb5ef0 =  6:  cdbb5ef0 lib/gshadow.c: sgetsgent(): Remove superfluous condition
 7:  4dc64ba1 =  7:  4dc64ba1 lib/gshadow.c: Move zeroing to within build_list()
 8:  91a20ba9 =  8:  91a20ba9 lib/gshadow.c: build_list(): Minimize use of pointer parameters
 9:  1c82588c =  9:  1c82588c lib/gshadow.c: build_list(): Compact ++ into previous statement
10:  cd30da3d = 10:  cd30da3d lib/gshadow.c: Remove dead code
11:  a1db95c8 = 11:  a1db95c8 lib/gshadow.c: sgetsgent(): Be consistent using NULL
12:  8d686837 = 12:  8d686837 lib/gshadow.c: build_list(): Remove second parameter
13:  cb12f334 = 13:  cb12f334 lib/gshadow.c: Remove redundant variables
14:  ea50249d = 14:  ea50249d lib/string/strchr/: strchrcnt(): Add function
15:  4d9d6c92 = 15:  4d9d6c92 lib/gshadow.c: build_list(): Allocate at once
16:  d5218548 = 16:  d5218548 lib/gshadow.c: build_list(): Transform while loop into for loop
 -:  -------- > 17:  ebcb371b lib/gshadow.c: Don't exit(3) on error in library code

v7b

Fix includes in intermediate commits.

$ git range-diff master gh/S S 
 1:  a84dfc09 =  1:  a84dfc09 src/: Transform do-while into while
 2:  215be064 =  2:  215be064 lib/gshadow.c: build_list(): Fix type of parameter
 3:  a8d604d9 =  3:  a8d604d9 lib/gshadow.c: build_list(): Remove unused variable
 4:  8ee64c91 =  4:  8ee64c91 lib/gshadow.c: build_list(): Improve variable and parameter names
 5:  53f8bfb0 =  5:  53f8bfb0 lib/gshadow.c: build_list(): Remove dead assignment
 6:  cdbb5ef0 =  6:  cdbb5ef0 lib/gshadow.c: sgetsgent(): Remove superfluous condition
 7:  4dc64ba1 =  7:  4dc64ba1 lib/gshadow.c: Move zeroing to within build_list()
 8:  91a20ba9 =  8:  91a20ba9 lib/gshadow.c: build_list(): Minimize use of pointer parameters
 9:  1c82588c =  9:  1c82588c lib/gshadow.c: build_list(): Compact ++ into previous statement
10:  cd30da3d = 10:  cd30da3d lib/gshadow.c: Remove dead code
11:  a1db95c8 = 11:  a1db95c8 lib/gshadow.c: sgetsgent(): Be consistent using NULL
12:  8d686837 = 12:  8d686837 lib/gshadow.c: build_list(): Remove second parameter
13:  cb12f334 = 13:  cb12f334 lib/gshadow.c: Remove redundant variables
14:  ea50249d = 14:  ea50249d lib/string/strchr/: strchrcnt(): Add function
15:  4d9d6c92 ! 15:  72f8b5d2 lib/gshadow.c: build_list(): Allocate at once
    @@ Commit message
     
      ## lib/gshadow.c ##
     @@
    - #include "alloc/x/xrealloc.h"
    + 
    + #include "alloc/malloc.h"
    + #include "alloc/realloc.h"
    +-#include "alloc/x/xrealloc.h"
    ++#include "alloc/x/xmalloc.h"
      #include "defines.h"
      #include "prototypes.h"
     +#include "string/strchr/strchrcnt.h"
16:  d5218548 = 16:  c45c6e6c lib/gshadow.c: build_list(): Transform while loop into for loop
17:  ebcb371b ! 17:  31359462 lib/gshadow.c: Don't exit(3) on error in library code
    @@ lib/gshadow.c
      
      #include "alloc/malloc.h"
      #include "alloc/realloc.h"
    --#include "alloc/x/xrealloc.h"
    +-#include "alloc/x/xmalloc.h"
      #include "defines.h"
      #include "prototypes.h"
      #include "string/strchr/strchrcnt.h"

$ git diff ebcb371b..31359462
$

v8

Fix double-free introduced in v7.

$ git range-diff master gh/S S 
 1:  a84dfc09 =  1:  a84dfc09 src/: Transform do-while into while
 2:  215be064 =  2:  215be064 lib/gshadow.c: build_list(): Fix type of parameter
 3:  a8d604d9 =  3:  a8d604d9 lib/gshadow.c: build_list(): Remove unused variable
 4:  8ee64c91 =  4:  8ee64c91 lib/gshadow.c: build_list(): Improve variable and parameter names
 5:  53f8bfb0 =  5:  53f8bfb0 lib/gshadow.c: build_list(): Remove dead assignment
 6:  cdbb5ef0 =  6:  cdbb5ef0 lib/gshadow.c: sgetsgent(): Remove superfluous condition
 7:  4dc64ba1 =  7:  4dc64ba1 lib/gshadow.c: Move zeroing to within build_list()
 8:  91a20ba9 =  8:  91a20ba9 lib/gshadow.c: build_list(): Minimize use of pointer parameters
 9:  1c82588c =  9:  1c82588c lib/gshadow.c: build_list(): Compact ++ into previous statement
10:  cd30da3d = 10:  cd30da3d lib/gshadow.c: Remove dead code
11:  a1db95c8 = 11:  a1db95c8 lib/gshadow.c: sgetsgent(): Be consistent using NULL
12:  8d686837 = 12:  8d686837 lib/gshadow.c: build_list(): Remove second parameter
13:  cb12f334 = 13:  cb12f334 lib/gshadow.c: Remove redundant variables
14:  ea50249d = 14:  ea50249d lib/string/strchr/: strchrcnt(): Add function
15:  72f8b5d2 = 15:  72f8b5d2 lib/gshadow.c: build_list(): Allocate at once
16:  c45c6e6c = 16:  c45c6e6c lib/gshadow.c: build_list(): Transform while loop into for loop
17:  31359462 ! 17:  28e8a8c8 lib/gshadow.c: Don't exit(3) on error in library code
    @@ lib/gshadow.c: build_list(char *s)
        for (i = 0; s != NULL && *s != '\0'; i++)
                l[i] = strsep(&s, ",");
     @@ lib/gshadow.c: sgetsgent(const char *string)
    -   free(sgroup.sg_mem);
    +   sgroup.sg_passwd = fields[1];
      
    +   free(sgroup.sg_adm);
    +-  free(sgroup.sg_mem);
    +-
        sgroup.sg_adm = build_list(fields[2]);
     +  if (sgroup.sg_adm == NULL)
     +          return NULL;
     +
    ++  free(sgroup.sg_mem);
        sgroup.sg_mem = build_list(fields[3]);
     +  if (sgroup.sg_mem == NULL)
    -+          goto fail;
    ++          return NULL;
      
        return &sgroup;
    -+
    -+fail:
    -+  free(sgroup.sg_adm);
    -+  return NULL;
      }
    - 
    - /*

v8b

Expand commit message

$ git range-diff master gh/S S 
 1:  a84dfc09 !  1:  61cb7718 src/: Transform do-while into while
    @@ Commit message
     
         list cannot be NULL in the first iteration, so we don't need a do-while.
     
    +    Just in case it's not obvious: we know it's not NULL in the first
    +    iteration because right above, in line 772, we've already dereferenced
    +    it.
    +
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## src/useradd.c ##
 2:  215be064 =  2:  c4e79ef2 lib/gshadow.c: build_list(): Fix type of parameter
 3:  a8d604d9 =  3:  884dec6c lib/gshadow.c: build_list(): Remove unused variable
 4:  8ee64c91 =  4:  1dda9f80 lib/gshadow.c: build_list(): Improve variable and parameter names
 5:  53f8bfb0 =  5:  30f7a5ac lib/gshadow.c: build_list(): Remove dead assignment
 6:  cdbb5ef0 =  6:  46c5277d lib/gshadow.c: sgetsgent(): Remove superfluous condition
 7:  4dc64ba1 =  7:  797843fb lib/gshadow.c: Move zeroing to within build_list()
 8:  91a20ba9 =  8:  81548993 lib/gshadow.c: build_list(): Minimize use of pointer parameters
 9:  1c82588c =  9:  c7f8b1d8 lib/gshadow.c: build_list(): Compact ++ into previous statement
10:  cd30da3d = 10:  397350a7 lib/gshadow.c: Remove dead code
11:  a1db95c8 = 11:  ae426619 lib/gshadow.c: sgetsgent(): Be consistent using NULL
12:  8d686837 = 12:  d13cb58d lib/gshadow.c: build_list(): Remove second parameter
13:  cb12f334 = 13:  2aeb4fbf lib/gshadow.c: Remove redundant variables
14:  ea50249d = 14:  20717f61 lib/string/strchr/: strchrcnt(): Add function
15:  72f8b5d2 = 15:  838f15a8 lib/gshadow.c: build_list(): Allocate at once
16:  c45c6e6c = 16:  fbaeb101 lib/gshadow.c: build_list(): Transform while loop into for loop
17:  28e8a8c8 = 17:  5942cc51 lib/gshadow.c: Don't exit(3) on error in library code

v8c

Reorder commits. This results in one line being removed earlier, and so we don't change it in an intermediate commit. That results in smaller intermediate diffs, and thus in a slightly easier review.
Pedantic wording tweaks to commit messages.

$ git range-diff master gh/S S 
 1:  61cb7718 =  1:  61cb7718 src/: Transform do-while into while
 2:  c4e79ef2 =  2:  c4e79ef2 lib/gshadow.c: build_list(): Fix type of parameter
 3:  884dec6c =  3:  884dec6c lib/gshadow.c: build_list(): Remove unused variable
 4:  1dda9f80 =  4:  1dda9f80 lib/gshadow.c: build_list(): Improve variable and parameter names
 5:  30f7a5ac =  5:  30f7a5ac lib/gshadow.c: build_list(): Remove dead assignment
 6:  46c5277d =  6:  46c5277d lib/gshadow.c: sgetsgent(): Remove superfluous condition
 7:  797843fb !  7:  24cdf7ac lib/gshadow.c: Move zeroing to within build_list()
    @@ Commit message
         lib/gshadow.c: Move zeroing to within build_list()
     
         This makes build_list() less dependent on the context.
    -    It starts from clean, whatever was the state before the call.
    +    It starts from clean, whatever the state before the call was.
         I was having a hard time understanding the reallocation,
         until I saw that we were zeroing everything right before the call.
     
10:  397350a7 !  8:  71a1f2bc lib/gshadow.c: Remove dead code
    @@ lib/gshadow.c
        char    **l;
        size_t  n;
     @@ lib/gshadow.c: build_list(char *s, char ***lp, size_t *np)
    -   l[n] = NULL;
    +           l[n] = strsep(&s, ",");
    +           n++;
    +           *lp = l;
    +-          *np = n;
    +   }
      
    -   *lp = l;
    --  *np = n;
    - 
    -   return l;
    - }
    +   l = XREALLOC(*lp, n + 1, char *);
     @@ lib/gshadow.c: sgetsgent(const char *string)
        free (admins);
        free (members);
 8:  81548993 !  9:  8de84242 lib/gshadow.c: build_list(): Minimize use of pointer parameters
    @@ Commit message
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## lib/gshadow.c ##
    -@@ lib/gshadow.c: build_list(char *s, char ***lp, size_t *np)
    +@@ lib/gshadow.c: build_list(char *s, char ***lp)
        char    **l;
        size_t  n;
      
    @@ lib/gshadow.c: build_list(char *s, char ***lp, size_t *np)
                l[n] = strsep(&s, ",");
                n++;
     -          *lp = l;
    --          *np = n;
        }
      
     -  l = XREALLOC(*lp, n + 1, char *);
    @@ lib/gshadow.c: build_list(char *s, char ***lp, size_t *np)
        l[n] = NULL;
     +
        *lp = l;
    -+  *np = n;
      
        return l;
    - }
 9:  c7f8b1d8 ! 10:  ffcf965e lib/gshadow.c: build_list(): Compact ++ into previous statement
    @@ Commit message
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## lib/gshadow.c ##
    -@@ lib/gshadow.c: build_list(char *s, char ***lp, size_t *np)
    +@@ lib/gshadow.c: build_list(char *s, char ***lp)
      
        while (s != NULL && *s != '\0') {
                l = XREALLOC(l, n + 1, char *);
11:  ae426619 = 11:  ba696b1e lib/gshadow.c: sgetsgent(): Be consistent using NULL
12:  d13cb58d ! 12:  485a5360 lib/gshadow.c: build_list(): Remove second parameter
    @@ Commit message
         value that the function returns.  It's simpler if the caller just sets
         it itself after the call.
     
    -    This removes the only 3-star pointer type declaration in the entire
    -    project.  :)
    +    This removes the only 3-star pointer in the entire project.  :)
     
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
13:  2aeb4fbf = 13:  f0fe1cdc lib/gshadow.c: Remove redundant variables
14:  20717f61 = 14:  04b23bef lib/string/strchr/: strchrcnt(): Add function
15:  838f15a8 = 15:  ebe83f4c lib/gshadow.c: build_list(): Allocate at once
16:  fbaeb101 = 16:  d7401353 lib/gshadow.c: build_list(): Transform while loop into for loop
17:  5942cc51 = 17:  0a0e877b lib/gshadow.c: Don't exit(3) on error in library code

$ git diff 5942cc51..0a0e877b
$

$ git lgo master..gh/S --stat | grep file.*changed | grep -o ',.*' | awk '{print $2 " " $4}' | tr '\n' ' ' | sed 's/ $//' | sed 's/ / + /g' | xargs expr
199
$ git lgo master..S --stat | grep file.*changed | grep -o ',.*' | awk '{print $2 " " $4}' | tr '\n' ' ' | sed 's/ $//' | sed 's/ / + /g' | xargs expr
197

v9

Use streq() [CC @stoeckmann ]

$ git range-diff master gh/S S 
 1:  61cb7718 =  1:  61cb7718 src/: Transform do-while into while
 2:  c4e79ef2 =  2:  c4e79ef2 lib/gshadow.c: build_list(): Fix type of parameter
 3:  884dec6c =  3:  884dec6c lib/gshadow.c: build_list(): Remove unused variable
 4:  1dda9f80 =  4:  1dda9f80 lib/gshadow.c: build_list(): Improve variable and parameter names
 5:  30f7a5ac =  5:  30f7a5ac lib/gshadow.c: build_list(): Remove dead assignment
 6:  46c5277d =  6:  46c5277d lib/gshadow.c: sgetsgent(): Remove superfluous condition
 7:  24cdf7ac =  7:  24cdf7ac lib/gshadow.c: Move zeroing to within build_list()
 8:  71a1f2bc =  8:  71a1f2bc lib/gshadow.c: Remove dead code
 9:  8de84242 =  9:  8de84242 lib/gshadow.c: build_list(): Minimize use of pointer parameters
10:  ffcf965e = 10:  ffcf965e lib/gshadow.c: build_list(): Compact ++ into previous statement
11:  ba696b1e = 11:  ba696b1e lib/gshadow.c: sgetsgent(): Be consistent using NULL
12:  485a5360 = 12:  485a5360 lib/gshadow.c: build_list(): Remove second parameter
13:  f0fe1cdc = 13:  f0fe1cdc lib/gshadow.c: Remove redundant variables
 -:  -------- > 14:  26777b3f lib/string/strcmp/: streq(): Add function
14:  04b23bef ! 15:  07927dd5 lib/string/strchr/: strchrcnt(): Add function
    @@ lib/string/strchr/strchrcnt.h (new)
     +#include <config.h>
     +
     +#include <stddef.h>
    -+#include <string.h>
     +
     +#include "attr.h"
    ++#include "string/strcmp/streq.h"
     +
     +
     +ATTR_STRING(1)
    @@ lib/string/strchr/strchrcnt.h (new)
     +{
     +  size_t  n = 0;
     +
    -+  for (; strcmp(s, "") != 0; s++) {
    ++  for (; !streq(s, ""); s++) {
     +          if (*s == c)
     +                  n++;
     +  }
15:  ebe83f4c = 16:  bb131063 lib/gshadow.c: build_list(): Allocate at once
16:  d7401353 = 17:  adecf632 lib/gshadow.c: build_list(): Transform while loop into for loop
17:  0a0e877b = 18:  f30e83ab lib/gshadow.c: Don't exit(3) on error in library code

v9b

Fix path of streq.[ch]

$ git range-diff master gh/S S 
 1:  61cb7718 =  1:  61cb7718 src/: Transform do-while into while
 2:  c4e79ef2 =  2:  c4e79ef2 lib/gshadow.c: build_list(): Fix type of parameter
 3:  884dec6c =  3:  884dec6c lib/gshadow.c: build_list(): Remove unused variable
 4:  1dda9f80 =  4:  1dda9f80 lib/gshadow.c: build_list(): Improve variable and parameter names
 5:  30f7a5ac =  5:  30f7a5ac lib/gshadow.c: build_list(): Remove dead assignment
 6:  46c5277d =  6:  46c5277d lib/gshadow.c: sgetsgent(): Remove superfluous condition
 7:  24cdf7ac =  7:  24cdf7ac lib/gshadow.c: Move zeroing to within build_list()
 8:  71a1f2bc =  8:  71a1f2bc lib/gshadow.c: Remove dead code
 9:  8de84242 =  9:  8de84242 lib/gshadow.c: build_list(): Minimize use of pointer parameters
10:  ffcf965e = 10:  ffcf965e lib/gshadow.c: build_list(): Compact ++ into previous statement
11:  ba696b1e = 11:  ba696b1e lib/gshadow.c: sgetsgent(): Be consistent using NULL
12:  485a5360 = 12:  485a5360 lib/gshadow.c: build_list(): Remove second parameter
13:  f0fe1cdc = 13:  f0fe1cdc lib/gshadow.c: Remove redundant variables
14:  26777b3f ! 14:  cb766bb4 lib/string/strcmp/: streq(): Add function
    @@ lib/Makefile.am: libshadow_la_SOURCES = \
        string/strcpy/stpecpy.h \
        string/strcpy/strncat.c \
     
    - ## lib/string/strchr/streq.c (new) ##
    + ## lib/string/strcmp/streq.c (new) ##
     @@
     +// SPDX-FileCopyrightText: 2024, Alejandro Colomar <alx@kernel.org>
     +// SPDX-License-Identifier: BSD-3-Clause
    @@ lib/string/strchr/streq.c (new)
     +
     +extern inline bool streq(const char *s1, const char *s2);
     
    - ## lib/string/strchr/streq.h (new) ##
    + ## lib/string/strcmp/streq.h (new) ##
     @@
     +// SPDX-FileCopyrightText: 2024, Alejandro Colomar <alx@kernel.org>
     +// SPDX-License-Identifier: BSD-3-Clause
15:  07927dd5 = 15:  8f568222 lib/string/strchr/: strchrcnt(): Add function
16:  bb131063 = 16:  a5e33f22 lib/gshadow.c: build_list(): Allocate at once
17:  adecf632 = 17:  a0647d2a lib/gshadow.c: build_list(): Transform while loop into for loop
18:  f30e83ab = 18:  483bd5f9 lib/gshadow.c: Don't exit(3) on error in library code

v10

Drop the last patch. We need to exit(3) on error. NULL is already a valid return value, so we can't use it to signal errors. This reverts the change introduced in v7. (But it's not a rollback, because the changes from v9 are good.)

$ git range-diff alx/master gh/S S
 1:  61cb7718 =  1:  61cb7718 src/: Transform do-while into while
 2:  c4e79ef2 =  2:  c4e79ef2 lib/gshadow.c: build_list(): Fix type of parameter
 3:  884dec6c =  3:  884dec6c lib/gshadow.c: build_list(): Remove unused variable
 4:  1dda9f80 =  4:  1dda9f80 lib/gshadow.c: build_list(): Improve variable and parameter names
 5:  30f7a5ac =  5:  30f7a5ac lib/gshadow.c: build_list(): Remove dead assignment
 6:  46c5277d =  6:  46c5277d lib/gshadow.c: sgetsgent(): Remove superfluous condition
 7:  24cdf7ac =  7:  24cdf7ac lib/gshadow.c: Move zeroing to within build_list()
 8:  71a1f2bc =  8:  71a1f2bc lib/gshadow.c: Remove dead code
 9:  8de84242 =  9:  8de84242 lib/gshadow.c: build_list(): Minimize use of pointer parameters
10:  ffcf965e = 10:  ffcf965e lib/gshadow.c: build_list(): Compact ++ into previous statement
11:  ba696b1e = 11:  ba696b1e lib/gshadow.c: sgetsgent(): Be consistent using NULL
12:  485a5360 = 12:  485a5360 lib/gshadow.c: build_list(): Remove second parameter
13:  f0fe1cdc = 13:  f0fe1cdc lib/gshadow.c: Remove redundant variables
14:  cb766bb4 = 14:  cb766bb4 lib/string/strcmp/: streq(): Add function
15:  8f568222 = 15:  8f568222 lib/string/strchr/: strchrcnt(): Add function
16:  a5e33f22 = 16:  a5e33f22 lib/gshadow.c: build_list(): Allocate at once
17:  a0647d2a = 17:  a0647d2a lib/gshadow.c: build_list(): Transform while loop into for loop
18:  483bd5f9 <  -:  -------- lib/gshadow.c: Don't exit(3) on error in library code

v10b

Rebase

$ git range-diff alx/master..gh/S shadow/master..S
 1:  61cb7718 =  1:  d60bde48 src/: Transform do-while into while
 2:  c4e79ef2 =  2:  b40fea2a lib/gshadow.c: build_list(): Fix type of parameter
 3:  884dec6c =  3:  b59f38ac lib/gshadow.c: build_list(): Remove unused variable
 4:  1dda9f80 =  4:  39296263 lib/gshadow.c: build_list(): Improve variable and parameter names
 5:  30f7a5ac =  5:  c7cbcfdf lib/gshadow.c: build_list(): Remove dead assignment
 6:  46c5277d =  6:  48183a6a lib/gshadow.c: sgetsgent(): Remove superfluous condition
 7:  24cdf7ac =  7:  f597893f lib/gshadow.c: Move zeroing to within build_list()
 8:  71a1f2bc =  8:  081b85c5 lib/gshadow.c: Remove dead code
 9:  8de84242 =  9:  b5541dd6 lib/gshadow.c: build_list(): Minimize use of pointer parameters
10:  ffcf965e = 10:  938c66d4 lib/gshadow.c: build_list(): Compact ++ into previous statement
11:  ba696b1e = 11:  3458f30c lib/gshadow.c: sgetsgent(): Be consistent using NULL
12:  485a5360 = 12:  06f74873 lib/gshadow.c: build_list(): Remove second parameter
13:  f0fe1cdc = 13:  b1ed3bf0 lib/gshadow.c: Remove redundant variables
14:  cb766bb4 <  -:  -------- lib/string/strcmp/: streq(): Add function
15:  8f568222 <  -:  -------- lib/string/strchr/: strchrcnt(): Add function
16:  a5e33f22 = 14:  7a767918 lib/gshadow.c: build_list(): Allocate at once
17:  a0647d2a = 15:  1d7cb9a0 lib/gshadow.c: build_list(): Transform while loop into for loop

v10c

Rebase

$ git range-diff gh/master..gh/S shadow/master..S
 1:  d60bde48 =  1:  29df81ea src/: Transform do-while into while
 2:  b40fea2a =  2:  1899e8bd lib/gshadow.c: build_list(): Fix type of parameter
 3:  b59f38ac =  3:  87526136 lib/gshadow.c: build_list(): Remove unused variable
 4:  39296263 =  4:  2270254f lib/gshadow.c: build_list(): Improve variable and parameter names
 5:  c7cbcfdf =  5:  b820b6a7 lib/gshadow.c: build_list(): Remove dead assignment
 6:  48183a6a =  6:  9a9e3ece lib/gshadow.c: sgetsgent(): Remove superfluous condition
 7:  f597893f =  7:  fe4ccf25 lib/gshadow.c: Move zeroing to within build_list()
 8:  081b85c5 =  8:  0b03349b lib/gshadow.c: Remove dead code
 9:  b5541dd6 =  9:  14e8e756 lib/gshadow.c: build_list(): Minimize use of pointer parameters
10:  938c66d4 = 10:  edee69c7 lib/gshadow.c: build_list(): Compact ++ into previous statement
11:  3458f30c = 11:  64df4f12 lib/gshadow.c: sgetsgent(): Be consistent using NULL
12:  06f74873 = 12:  5bee2e90 lib/gshadow.c: build_list(): Remove second parameter
13:  b1ed3bf0 = 13:  6280a334 lib/gshadow.c: Remove redundant variables
14:  7a767918 = 14:  6ed420d8 lib/gshadow.c: build_list(): Allocate at once
15:  1d7cb9a0 = 15:  8c81bc65 lib/gshadow.c: build_list(): Transform while loop into for loop

v10d

Rebase

$ git range-diff master..gh/S shadow/master..S
 1:  29df81ea =  1:  70a2286d src/: Transform do-while into while
 2:  1899e8bd =  2:  9903e09e lib/gshadow.c: build_list(): Fix type of parameter
 3:  87526136 =  3:  257487c5 lib/gshadow.c: build_list(): Remove unused variable
 4:  2270254f =  4:  f024f1e1 lib/gshadow.c: build_list(): Improve variable and parameter names
 5:  b820b6a7 =  5:  0976f283 lib/gshadow.c: build_list(): Remove dead assignment
 6:  9a9e3ece =  6:  485f8ffe lib/gshadow.c: sgetsgent(): Remove superfluous condition
 7:  fe4ccf25 =  7:  9eb4fbcd lib/gshadow.c: Move zeroing to within build_list()
 8:  0b03349b =  8:  a28ff857 lib/gshadow.c: Remove dead code
 9:  14e8e756 =  9:  7ff9d3ee lib/gshadow.c: build_list(): Minimize use of pointer parameters
10:  edee69c7 = 10:  8ef538fa lib/gshadow.c: build_list(): Compact ++ into previous statement
11:  64df4f12 = 11:  a5531e54 lib/gshadow.c: sgetsgent(): Be consistent using NULL
12:  5bee2e90 = 12:  a9644750 lib/gshadow.c: build_list(): Remove second parameter
13:  6280a334 = 13:  17749815 lib/gshadow.c: Remove redundant variables
14:  6ed420d8 ! 14:  247ed87f lib/gshadow.c: build_list(): Allocate at once
    @@ lib/gshadow.c
      #include "defines.h"
      #include "prototypes.h"
     +#include "string/strchr/strchrcnt.h"
    + #include "string/strcmp/streq.h"
      #include "string/strtok/stpsep.h"
      
    - 
     @@ lib/gshadow.c: build_list(char *s)
        char    **l;
        size_t  n;
15:  8c81bc65 = 15:  74ded0eb lib/gshadow.c: build_list(): Transform while loop into for loop

v10e

Rebase

$ git range-diff master..gh/S shadow/master..S 
 1:  70a2286d =  1:  365640ca src/: Transform do-while into while
 2:  9903e09e =  2:  c2297d20 lib/gshadow.c: build_list(): Fix type of parameter
 3:  257487c5 =  3:  f531eb2e lib/gshadow.c: build_list(): Remove unused variable
 4:  f024f1e1 =  4:  de66a1a7 lib/gshadow.c: build_list(): Improve variable and parameter names
 5:  0976f283 =  5:  f3aa5bc5 lib/gshadow.c: build_list(): Remove dead assignment
 6:  485f8ffe =  6:  0316dfc1 lib/gshadow.c: sgetsgent(): Remove superfluous condition
 7:  9eb4fbcd =  7:  e17d923c lib/gshadow.c: Move zeroing to within build_list()
 8:  a28ff857 =  8:  777a624f lib/gshadow.c: Remove dead code
 9:  7ff9d3ee =  9:  e9d3cc37 lib/gshadow.c: build_list(): Minimize use of pointer parameters
10:  8ef538fa = 10:  71aca593 lib/gshadow.c: build_list(): Compact ++ into previous statement
11:  a5531e54 = 11:  43001e03 lib/gshadow.c: sgetsgent(): Be consistent using NULL
12:  a9644750 = 12:  45c4f316 lib/gshadow.c: build_list(): Remove second parameter
13:  17749815 = 13:  20ac1b8e lib/gshadow.c: Remove redundant variables
14:  247ed87f = 14:  674dacf0 lib/gshadow.c: build_list(): Allocate at once
15:  74ded0eb = 15:  e70946e4 lib/gshadow.c: build_list(): Transform while loop into for loop

v10f

Rebase

$ git range-diff master..gh/S shadow/master..S 
 1:  365640ca =  1:  1b5db257 src/: Transform do-while into while
 2:  c2297d20 =  2:  c99b1775 lib/gshadow.c: build_list(): Fix type of parameter
 3:  f531eb2e =  3:  68ba4d4d lib/gshadow.c: build_list(): Remove unused variable
 4:  de66a1a7 =  4:  e90e5b8f lib/gshadow.c: build_list(): Improve variable and parameter names
 5:  f3aa5bc5 =  5:  16ad8717 lib/gshadow.c: build_list(): Remove dead assignment
 6:  0316dfc1 =  6:  6861fb6a lib/gshadow.c: sgetsgent(): Remove superfluous condition
 7:  e17d923c =  7:  cfe91319 lib/gshadow.c: Move zeroing to within build_list()
 8:  777a624f =  8:  1c994208 lib/gshadow.c: Remove dead code
 9:  e9d3cc37 =  9:  38da389d lib/gshadow.c: build_list(): Minimize use of pointer parameters
10:  71aca593 = 10:  e016183c lib/gshadow.c: build_list(): Compact ++ into previous statement
11:  43001e03 = 11:  95b91dea lib/gshadow.c: sgetsgent(): Be consistent using NULL
12:  45c4f316 = 12:  ed5935cb lib/gshadow.c: build_list(): Remove second parameter
13:  20ac1b8e = 13:  3da8398e lib/gshadow.c: Remove redundant variables
14:  674dacf0 = 14:  62716830 lib/gshadow.c: build_list(): Allocate at once
15:  e70946e4 = 15:  6e8d761d lib/gshadow.c: build_list(): Transform while loop into for loop

alejandro-colomar · 2024-11-04T21:27:01Z

It's amazing how much could be removed just from that function. :)

src/useradd.c

alejandro-colomar · 2024-11-05T14:17:29Z

I do not be a 3-star programmer! :-)

commit 1abba41f37c14187cff53892300a7be49b781c5e (HEAD -> S, gh/S)
Author: Alejandro Colomar <alx@kernel.org>
Date:   Tue Nov 5 15:13:35 2024 +0100

    lib/gshadow.c: build_list(): Remove second parameter
    
    We've simplified the function so much in the previous commits, that now
    $2 is rather useless.  It only sets the output parameter to the same
    value that the function returns.  It's simpler if the caller just sets
    it itself after the call.
    
    This removes the only 3-star pointer type declaration in the entire
    project.  :)
    
    Signed-off-by: Alejandro Colomar <alx@kernel.org>

diff --git a/lib/gshadow.c b/lib/gshadow.c
index 5caf4f16..7de52aaf 100644
--- a/lib/gshadow.c
+++ b/lib/gshadow.c
@@ -35,7 +35,7 @@ static struct sgrp sgroup;
 
 
 static /*@null@*/char **
-build_list(char *s, char ***lp)
+build_list(char *s)
 {
        char    **l;
        size_t  n;
@@ -51,8 +51,6 @@ build_list(char *s, char ***lp)
        l = XREALLOC(l, n + 1, char *);
        l[n] = NULL;
 
-       *lp = l;
-
        return l;
 }
 
@@ -119,8 +117,8 @@ sgetsgent(const char *string)
        free(admins);
        free(members);
 
-       sgroup.sg_adm = build_list(fields[2], &admins);
-       sgroup.sg_mem = build_list(fields[3], &members);
+       sgroup.sg_adm = admins  = build_list(fields[2]);
+       sgroup.sg_mem = members = build_list(fields[3]);
 
        return &sgroup;
 }

alejandro-colomar · 2024-11-06T17:54:24Z

Il semble que la perfection soit atteinte non quand il n'y a plus rien à ajouter, mais quand il n'y a plus rien à retrancher.

Antoine de Saint Exupéry

lib/string/strchr/strchrcnt.h

list cannot be NULL in the first iteration, so we don't need a do-while. Just in case it's not obvious: we know it's not NULL in the first iteration because right above, in line 772, we've already dereferenced it. Signed-off-by: Alejandro Colomar <alx@kernel.org>

@null

list ($2) is a pointer to a list of strings. We were declaring it as an array of pointers to strings, which was bogus. It worked out of luck, because array parameters are transformed into pointers by the compiler, but it was incorrect. Just look at how we're calling this function. $ grep build_list lib/gshadow.c build_list(char *s, char ***list, size_t *nlist) sgroup.sg_adm = build_list (fields[2], &admins, &nadmins); sgroup.sg_mem = build_list (fields[3], &members, &nmembers); $ grep '^static .*\<admins\>' lib/gshadow.c static /*@null@*//*@only@*/char **admins = NULL; $ grep '^static .*\<members\>' lib/gshadow.c static /*@null@*//*@only@*/char **members = NULL; Fixes: 8e167d2 ("[svn-upgrade] Integrating new upstream version, shadow (4.0.8)") Signed-off-by: Alejandro Colomar <alx@kernel.org>

Signed-off-by: Alejandro Colomar <alx@kernel.org>

It was hard to understand what each variable is. Use a consistent scheme, where a 'p' means a pointer, 'l' means list, and 'n' means number of elements. Those should be obvious from the name of the function and the context, and will make it easier to read the code. Also, the shorter names will allow focusing on the rest of the code. Signed-off-by: Alejandro Colomar <alx@kernel.org>

Signed-off-by: Alejandro Colomar <alx@kernel.org>

If n was 0, it doesn't hurt to set it again to 0; and the list would be NULL, so it doesn't hurt free(3)ing it and setting to NULL again either. Signed-off-by: Alejandro Colomar <alx@kernel.org>

This makes build_list() less dependent on the context. It starts from clean, whatever the state before the call was. I was having a hard time understanding the reallocation, until I saw that we were zeroing everything right before the call. Signed-off-by: Alejandro Colomar <alx@kernel.org>

Nothing is using that value outside of build_list(). Keep it as an local variable. Signed-off-by: Alejandro Colomar <alx@kernel.org>

Use instead automatic variables as much as possible. This reduces the number of dereferences, enhancing readability. Signed-off-by: Alejandro Colomar <alx@kernel.org>

Signed-off-by: Alejandro Colomar <alx@kernel.org>

0 is a horrible null-pointer constant. Don't use it. Especially, when just a few lines above, in the same function, we've used NULL for the same thing. Signed-off-by: Alejandro Colomar <alx@kernel.org>

We've simplified the function so much in the previous commits, that now $2 is rather useless. It only sets the output parameter to the same value that the function returns. It's simpler if the caller just sets it itself after the call. This removes the only 3-star pointer in the entire project. :) Signed-off-by: Alejandro Colomar <alx@kernel.org>

Signed-off-by: Alejandro Colomar <alx@kernel.org>

Instead of reallocating 1 more meber per iteration, calculate the total amount that we want by counting the number of commas (delimiters) in the string, plus one for the last element, plus one for the terminating NULL. This might result in overallocation of one element if the string is an empty string, or if there's a trailing comma; however, that's not an issue. We can afford overallocating one element in certain cases, and we get in exchange a much simpler function. Signed-off-by: Alejandro Colomar <alx@kernel.org>

And 'n' is now an iterator. Rename it to 'i' as usual. Signed-off-by: Alejandro Colomar <alx@kernel.org>

src/useradd.c

hallyn · 2024-12-06T03:21:16Z

I laughed, I cried, it was more suspenseful than war and peace.

alejandro-colomar · 2024-12-06T11:39:02Z

I laughed, I cried, it was more suspenseful than war and peace.

Heh, it was very fun to write this patch set/story. At any point you would believe that the function would be so simple after all. :-)

alejandro-colomar marked this pull request as ready for review November 4, 2024 17:29

alejandro-colomar added the Simpler A good issue for a new beginner label Nov 4, 2024

alejandro-colomar force-pushed the S branch from aaf77ac to 8c85292 Compare November 4, 2024 21:13

alejandro-colomar commented Nov 4, 2024

View reviewed changes

src/useradd.c Show resolved Hide resolved

alejandro-colomar force-pushed the S branch 2 times, most recently from ac58bc2 to 7622cc1 Compare November 5, 2024 14:05

alejandro-colomar requested review from hallyn and ikerexxe November 5, 2024 14:17

alejandro-colomar force-pushed the S branch from 1abba41 to cb12f33 Compare November 5, 2024 14:31

alejandro-colomar changed the title ~~Simplify some stuff, and add readability.~~ Simplify some stuff, enhance readability, and improve error handling. Nov 6, 2024

alejandro-colomar removed the Simpler A good issue for a new beginner label Nov 6, 2024

alejandro-colomar force-pushed the S branch 2 times, most recently from 3135946 to 28e8a8c Compare November 6, 2024 00:57

alejandro-colomar marked this pull request as draft November 6, 2024 11:12

alejandro-colomar marked this pull request as ready for review November 6, 2024 11:14

alejandro-colomar force-pushed the S branch from 28e8a8c to 5942cc5 Compare November 6, 2024 11:15

alejandro-colomar mentioned this pull request Nov 6, 2024

configure.ac, lib/, src/: Presume working shadow group support in libc #1111

Open

This comment was marked as outdated.

Sign in to view

alejandro-colomar closed this Nov 6, 2024

alejandro-colomar reopened this Nov 6, 2024

alejandro-colomar force-pushed the S branch from 5942cc5 to 0a0e877 Compare November 6, 2024 17:14

alejandro-colomar changed the title ~~Simplify some stuff, enhance readability, and improve error handling.~~ Simplify Nov 6, 2024

stoeckmann reviewed Nov 9, 2024

View reviewed changes

lib/string/strchr/strchrcnt.h Outdated Show resolved Hide resolved

alejandro-colomar force-pushed the S branch 2 times, most recently from f30e83a to 483bd5f Compare November 9, 2024 13:02

alejandro-colomar marked this pull request as draft November 10, 2024 13:48

alejandro-colomar force-pushed the S branch from 483bd5f to a0647d2 Compare November 10, 2024 13:50

alejandro-colomar marked this pull request as ready for review November 10, 2024 13:51

alejandro-colomar added the Simpler A good issue for a new beginner label Nov 10, 2024

alejandro-colomar force-pushed the S branch from a0647d2 to 1d7cb9a Compare November 11, 2024 09:52

alejandro-colomar mentioned this pull request Nov 13, 2024

4.17.0 release plan #1122

Open

alejandro-colomar force-pushed the S branch from 1d7cb9a to 8c81bc6 Compare November 13, 2024 17:33

alejandro-colomar force-pushed the S branch 2 times, most recently from 74ded0e to e70946e Compare December 5, 2024 15:17

alejandro-colomar added 15 commits December 6, 2024 00:47

lib/gshadow.c: build_list(): Remove unused variable

68ba4d4

Signed-off-by: Alejandro Colomar <alx@kernel.org>

lib/gshadow.c: build_list(): Remove dead assignment

16ad871

Signed-off-by: Alejandro Colomar <alx@kernel.org>

lib/gshadow.c: sgetsgent(): Remove superfluous condition

6861fb6

If n was 0, it doesn't hurt to set it again to 0; and the list would be NULL, so it doesn't hurt free(3)ing it and setting to NULL again either. Signed-off-by: Alejandro Colomar <alx@kernel.org>

lib/gshadow.c: Remove dead code

1c99420

Nothing is using that value outside of build_list(). Keep it as an local variable. Signed-off-by: Alejandro Colomar <alx@kernel.org>

lib/gshadow.c: build_list(): Minimize use of pointer parameters

38da389

Use instead automatic variables as much as possible. This reduces the number of dereferences, enhancing readability. Signed-off-by: Alejandro Colomar <alx@kernel.org>

lib/gshadow.c: build_list(): Compact ++ into previous statement

e016183

Signed-off-by: Alejandro Colomar <alx@kernel.org>

lib/gshadow.c: sgetsgent(): Be consistent using NULL

95b91de

0 is a horrible null-pointer constant. Don't use it. Especially, when just a few lines above, in the same function, we've used NULL for the same thing. Signed-off-by: Alejandro Colomar <alx@kernel.org>

lib/gshadow.c: Remove redundant variables

3da8398

Signed-off-by: Alejandro Colomar <alx@kernel.org>

lib/gshadow.c: build_list(): Transform while loop into for loop

6e8d761

And 'n' is now an iterator. Rename it to 'i' as usual. Signed-off-by: Alejandro Colomar <alx@kernel.org>

alejandro-colomar force-pushed the S branch from e70946e to 6e8d761 Compare December 5, 2024 23:48

hallyn approved these changes Dec 6, 2024

View reviewed changes

src/useradd.c Show resolved Hide resolved

hallyn merged commit 2f74389 into shadow-maint:master Dec 6, 2024
9 checks passed

alejandro-colomar deleted the S branch December 6, 2024 11:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplify #1109

Simplify #1109

alejandro-colomar commented Nov 4, 2024 •

edited

Loading

alejandro-colomar commented Nov 4, 2024 •

edited

Loading

alejandro-colomar commented Nov 5, 2024

This comment was marked as outdated.

alejandro-colomar commented Nov 6, 2024 •

edited

Loading

hallyn commented Dec 6, 2024

alejandro-colomar commented Dec 6, 2024 •

edited

Loading

Simplify #1109

Simplify #1109

Conversation

alejandro-colomar commented Nov 4, 2024 • edited Loading

alejandro-colomar commented Nov 4, 2024 • edited Loading

alejandro-colomar commented Nov 5, 2024

This comment was marked as outdated.

alejandro-colomar commented Nov 6, 2024 • edited Loading

hallyn commented Dec 6, 2024

alejandro-colomar commented Dec 6, 2024 • edited Loading

alejandro-colomar commented Nov 4, 2024 •

edited

Loading

alejandro-colomar commented Nov 4, 2024 •

edited

Loading

alejandro-colomar commented Nov 6, 2024 •

edited

Loading

alejandro-colomar commented Dec 6, 2024 •

edited

Loading