Skip to content

Conversation

@alejandro-colomar
Copy link
Collaborator

@alejandro-colomar alejandro-colomar commented Jun 10, 2025

I've managed to remove one of the static, but most of them seem necessary, since these shadow APIs are either libc APIs or similar to libc APIs, and they're specified as using static memory.

I've also done a lot of simplification and de-duplication work in surrounding code, especially in code transforming CSVs to lists.



Revisions:

v1b
  • Rebase
$ git rd 
 1:  bc196a4c =  1:  e030f9f9 lib/shadow/group/sgetgrent.c: Move free(3) call outside of helper
 2:  03fd5229 =  2:  922cd79c lib/shadow/gshadow/sgetsgent.c: Don't exit from library code
 3:  da4f193d =  3:  6087810b lib/, src/: Use more readable syntax for empty loops
 4:  969c53f6 =  4:  2b9b392d lib/: Deduplicate code
 5:  6bd67e17 =  5:  5a34d66d lib/list.c: comma_to_list(): Allow a trailing comma
 6:  140d5c09 =  6:  7ff2273b lib/list.c: comma_to_list(): Use xastrsep2ls() instead of its pattern
 7:  c1cf0676 =  7:  d6c5e912 lib/shadow/group/sgetgrent.c: Remove redundant code
 8:  09918ba6 =  8:  156085ca lib/list.c: comma_to_list(): Use build_list() instead of its pattern
 9:  06062942 =  9:  f1ee9cd3 lib/: Rename build_list() => acsv2ls()
10:  f0e13f5b = 10:  4f27d718 lib/, src/: Move list.c prototypes to new file list.h
v1c
  • Rebase
$ git rd 
 1:  e030f9f9 =  1:  fdba8427 lib/shadow/group/sgetgrent.c: Move free(3) call outside of helper
 2:  922cd79c =  2:  721a5846 lib/shadow/gshadow/sgetsgent.c: Don't exit from library code
 3:  6087810b =  3:  baac1e9f lib/, src/: Use more readable syntax for empty loops
 4:  2b9b392d =  4:  1b10b043 lib/: Deduplicate code
 5:  5a34d66d =  5:  56803058 lib/list.c: comma_to_list(): Allow a trailing comma
 6:  7ff2273b =  6:  2ad1c605 lib/list.c: comma_to_list(): Use xastrsep2ls() instead of its pattern
 7:  d6c5e912 =  7:  070df553 lib/shadow/group/sgetgrent.c: Remove redundant code
 8:  156085ca =  8:  5ae417e6 lib/list.c: comma_to_list(): Use build_list() instead of its pattern
 9:  f1ee9cd3 =  9:  f3e1a203 lib/: Rename build_list() => acsv2ls()
10:  4f27d718 = 10:  5ec6addd lib/, src/: Move list.c prototypes to new file list.h
v1d
  • Rebase
$ git rd 
 1:  fdba8427c =  1:  3ed5c07dd lib/shadow/group/sgetgrent.c: Move free(3) call outside of helper
 2:  721a5846d !  2:  a5f67fa47 lib/shadow/gshadow/sgetsgent.c: Don't exit from library code
    @@ Commit message
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## lib/shadow/gshadow/sgetsgent.c ##
    -@@
    - 
    - #include "shadow/gshadow/sgrp.h"
    - #include "string/strcmp/streq.h"
    -+#include "string/strtok/astrsep2ls.h"
    - #include "string/strtok/stpsep.h"
    - #include "string/strtok/strsep2arr.h"
    --#include "string/strtok/xastrsep2ls.h"
    - 
    - 
    - #if defined(SHADOWGRP) && !__has_include(<gshadow.h>)
     @@ lib/shadow/gshadow/sgetsgent.c: sgetsgent(const char *s)
        sgroup.sg_adm = build_list(fields[2]);
        sgroup.sg_mem = build_list(fields[3]);
 3:  baac1e9ff =  3:  b148e9e58 lib/, src/: Use more readable syntax for empty loops
 4:  1b10b0432 !  4:  35f0cf989 lib/: Deduplicate code
    @@ lib/list.c
     @@
      #include "string/strchr/strchrcnt.h"
      #include "string/strcmp/streq.h"
    - #include "string/strdup/xstrdup.h"
    + #include "string/strdup/strdup.h"
     +#include "string/strtok/astrsep2ls.h"
      #include "string/strtok/strsep2ls.h"
      
 5:  568030588 =  5:  aecafd272 lib/list.c: comma_to_list(): Allow a trailing comma
 6:  2ad1c6058 !  6:  dbaa72cad lib/list.c: comma_to_list(): Use xastrsep2ls() instead of its pattern
    @@ Commit message
     
      ## lib/list.c ##
     @@
    - #include "alloc/x/xmalloc.h"
    + #include "alloc/malloc.h"
      #include "prototypes.h"
      #include "defines.h"
     -#include "string/strchr/strchrcnt.h"
      #include "string/strcmp/streq.h"
    - #include "string/strdup/xstrdup.h"
    + #include "string/strdup/strdup.h"
      #include "string/strtok/astrsep2ls.h"
     -#include "string/strtok/strsep2ls.h"
     +#include "string/strtok/xastrsep2ls.h"
 7:  070df5533 =  7:  904bbf690 lib/shadow/group/sgetgrent.c: Remove redundant code
 8:  5ae417e6d !  8:  0e6a1845b lib/list.c: comma_to_list(): Use build_list() instead of its pattern
    @@ lib/list.c
      
     @@
      #include "string/strcmp/streq.h"
    - #include "string/strdup/xstrdup.h"
    + #include "string/strdup/strdup.h"
      #include "string/strtok/astrsep2ls.h"
     -#include "string/strtok/xastrsep2ls.h"
      
 9:  f3e1a203f =  9:  4e16128ca lib/: Rename build_list() => acsv2ls()
10:  5ec6adddb ! 10:  df842fb78 lib/, src/: Move list.c prototypes to new file list.h
    @@ lib/list.c
      
      #include <assert.h>
      
    - #include "alloc/x/xmalloc.h"
    + #include "alloc/malloc.h"
     -#include "prototypes.h"
      #include "defines.h"
      #include "string/strcmp/streq.h"
    - #include "string/strdup/xstrdup.h"
    + #include "string/strdup/strdup.h"
     
      ## lib/list.h (new) ##
     @@
    @@ src/groupadd.c
     
      ## src/groupmems.c ##
     @@
    - #include "alloc/x/xmalloc.h"
    + #include "alloc/malloc.h"
      #include "defines.h"
      #include "groupio.h"
     +#include "list.h"
v1e
  • Rebase
$ git rd 
 1:  3ed5c07dd =  1:  12ea33aa8 lib/shadow/group/sgetgrent.c: Move free(3) call outside of helper
 2:  a5f67fa47 =  2:  212fb4e94 lib/shadow/gshadow/sgetsgent.c: Don't exit from library code
 3:  b148e9e58 =  3:  cc2e96c87 lib/, src/: Use more readable syntax for empty loops
 4:  35f0cf989 =  4:  2b6b59b8c lib/: Deduplicate code
 5:  aecafd272 =  5:  aa2531ebc lib/list.c: comma_to_list(): Allow a trailing comma
 6:  dbaa72cad =  6:  521212d54 lib/list.c: comma_to_list(): Use xastrsep2ls() instead of its pattern
 7:  904bbf690 =  7:  b16de7a41 lib/shadow/group/sgetgrent.c: Remove redundant code
 8:  0e6a1845b =  8:  a41fd8c6a lib/list.c: comma_to_list(): Use build_list() instead of its pattern
 9:  4e16128ca =  9:  2c199ddda lib/: Rename build_list() => acsv2ls()
10:  df842fb78 = 10:  fcb515161 lib/, src/: Move list.c prototypes to new file list.h
v1f
  • Rebase
$ git rd 
 1:  12ea33aa8 =  1:  f2e930dab lib/shadow/group/sgetgrent.c: Move free(3) call outside of helper
 2:  212fb4e94 =  2:  e84fcbd2e lib/shadow/gshadow/sgetsgent.c: Don't exit from library code
 3:  cc2e96c87 =  3:  244d6c5a0 lib/, src/: Use more readable syntax for empty loops
 4:  2b6b59b8c =  4:  89a7b9d07 lib/: Deduplicate code
 5:  aa2531ebc =  5:  a136eb65a lib/list.c: comma_to_list(): Allow a trailing comma
 6:  521212d54 =  6:  1991a4d0f lib/list.c: comma_to_list(): Use xastrsep2ls() instead of its pattern
 7:  b16de7a41 =  7:  304937926 lib/shadow/group/sgetgrent.c: Remove redundant code
 8:  a41fd8c6a =  8:  bd643b492 lib/list.c: comma_to_list(): Use build_list() instead of its pattern
 9:  2c199ddda =  9:  63bc6eb32 lib/: Rename build_list() => acsv2ls()
10:  fcb515161 = 10:  989b2ab20 lib/, src/: Move list.c prototypes to new file list.h
v2
  • Rebase
$ git rd 
 1:  f2e930dab =  1:  e7e79c05a lib/shadow/group/sgetgrent.c: Move free(3) call outside of helper
 2:  e84fcbd2e =  2:  b7996af99 lib/shadow/gshadow/sgetsgent.c: Don't exit from library code
 3:  244d6c5a0 =  3:  9495b008a lib/, src/: Use more readable syntax for empty loops
 4:  89a7b9d07 !  4:  415754ce3 lib/: Deduplicate code
    @@ lib/list.c: comma_to_list(const char *comma)
     +}
     
      ## lib/prototypes.h ##
    -@@ lib/prototypes.h: extern /*@only@*/char **del_list (/*@returned@*/ /*@only@*/char **, const char *
    - extern /*@only@*/char **dup_list (char *const *);
    +@@ lib/prototypes.h: extern /*@only@*/char **dup_list (char *const *);
    + extern void free_list (char **);
      extern bool is_on_list (char *const *list, const char *member);
      extern /*@only@*/char **comma_to_list (const char *);
     +extern char **build_list(char *s);
 5:  a136eb65a =  5:  07ed7fb10 lib/list.c: comma_to_list(): Allow a trailing comma
 6:  1991a4d0f =  6:  751a277e5 lib/list.c: comma_to_list(): Use xastrsep2ls() instead of its pattern
 7:  304937926 !  7:  71ef200c0 lib/shadow/group/sgetgrent.c: Remove redundant code
    @@ lib/shadow/group/sgetgrent.c
      #include "string/strtok/strsep2arr.h"
      
     @@ lib/shadow/group/sgetgrent.c: sgetgrent(const char *s)
    -   if (STRSEP2ARR(dup, ":", fields) == -1)
    +   if (strsep2arr_a(dup, ":", fields) == -1)
                return NULL;
      
     -  if (streq(fields[2], ""))
 8:  bd643b492 =  8:  a3db219a5 lib/list.c: comma_to_list(): Use build_list() instead of its pattern
 9:  63bc6eb32 !  9:  ee367ee75 lib/: Rename build_list() => acsv2ls()
    @@ lib/list.c: comma_to_list(const char *comma)
        size_t  n;
     
      ## lib/prototypes.h ##
    -@@ lib/prototypes.h: extern /*@only@*/char **del_list (/*@returned@*/ /*@only@*/char **, const char *
    - extern /*@only@*/char **dup_list (char *const *);
    +@@ lib/prototypes.h: extern /*@only@*/char **dup_list (char *const *);
    + extern void free_list (char **);
      extern bool is_on_list (char *const *list, const char *member);
      extern /*@only@*/char **comma_to_list (const char *);
     -extern char **build_list(char *s);
10:  989b2ab20 ! 10:  a0b24e54b lib/, src/: Move list.c prototypes to new file list.h
    @@ lib/limits.c
     -#include "shadowlog.h"
      #include <sys/resource.h>
      
    - #include "atoi/a2i/a2i.h"
    - #include "atoi/a2i/a2s.h"
    - #include "atoi/str2i.h"
    + #include "atoi/a2i.h"
     +#include "defines.h"
     +#include "list.h"
     +#include "getdef.h"
    @@ lib/list.h (new)
     +extern /*@only@*/char **add_list (/*@returned@*/ /*@only@*/char **, const char *);
     +extern /*@only@*/char **del_list (/*@returned@*/ /*@only@*/char **, const char *);
     +extern /*@only@*/char **dup_list (char *const *);
    ++extern void free_list (char **);
     +extern bool is_on_list (char *const *list, const char *member);
     +extern /*@only@*/char **comma_to_list (const char *);
     +extern char **acsv2ls(char *s);
    @@ lib/prototypes.h: void audit_logger_with_group(int type, const char *op, const c
     -extern /*@only@*/char **add_list (/*@returned@*/ /*@only@*/char **, const char *);
     -extern /*@only@*/char **del_list (/*@returned@*/ /*@only@*/char **, const char *);
     -extern /*@only@*/char **dup_list (char *const *);
    +-extern void free_list (char **);
     -extern bool is_on_list (char *const *list, const char *member);
     -extern /*@only@*/char **comma_to_list (const char *);
     -extern char **acsv2ls(char *s);
    @@ lib/shadow/gshadow/sgetsgent.c
      ## src/chgpasswd.c ##
     @@
      #endif                            /* ACCT_TOOLS_SETUID */
    - #include "atoi/str2i.h"
    + #include "atoi/a2i.h"
      #include "defines.h"
     +#include "list.h"
      #include "nscd.h"
v2b
  • Rebase
$ git rd 
 1:  e7e79c05a =  1:  97b9ac63c lib/shadow/group/sgetgrent.c: Move free(3) call outside of helper
 2:  b7996af99 =  2:  999b32c17 lib/shadow/gshadow/sgetsgent.c: Don't exit from library code
 3:  9495b008a !  3:  85f64c986 lib/, src/: Use more readable syntax for empty loops
    @@ src/groupdel.c: static void group_busy (gid_t gid)
      
     
      ## src/logoutd.c ##
    -@@ src/logoutd.c: main(int argc, char **argv)
    +@@ src/logoutd.c: main(int argc, char *[])
        (void) textdomain (PACKAGE);
      
      #ifndef DEBUG
    @@ src/logoutd.c: main(int argc, char **argv)
      
        setpgrp ();
      
    -@@ src/logoutd.c: main(int argc, char **argv)
    +@@ src/logoutd.c: main(int argc, char *[])
                /*
                 * Reap any dead babies ...
                 */
 4:  415754ce3 =  4:  5e0eaa277 lib/: Deduplicate code
 5:  07ed7fb10 =  5:  2c42a8f69 lib/list.c: comma_to_list(): Allow a trailing comma
 6:  751a277e5 =  6:  fda4d2fd3 lib/list.c: comma_to_list(): Use xastrsep2ls() instead of its pattern
 7:  71ef200c0 =  7:  2317c26a0 lib/shadow/group/sgetgrent.c: Remove redundant code
 8:  a3db219a5 =  8:  e63a13df3 lib/list.c: comma_to_list(): Use build_list() instead of its pattern
 9:  ee367ee75 =  9:  05686fa62 lib/: Rename build_list() => acsv2ls()
10:  a0b24e54b ! 10:  7dd70a10c lib/, src/: Move list.c prototypes to new file list.h
    @@ src/groupadd.c
     
      ## src/groupmems.c ##
     @@
    - #include "alloc/malloc.h"
    + #include "attr.h"
      #include "defines.h"
      #include "groupio.h"
     +#include "list.h"
v3
  • Remove unused variable.
$ git rd 
 1:  97b9ac63c =  1:  97b9ac63c lib/shadow/group/sgetgrent.c: Move free(3) call outside of helper
 2:  999b32c17 =  2:  999b32c17 lib/shadow/gshadow/sgetsgent.c: Don't exit from library code
 3:  85f64c986 =  3:  85f64c986 lib/, src/: Use more readable syntax for empty loops
 4:  5e0eaa277 =  4:  5e0eaa277 lib/: Deduplicate code
 5:  2c42a8f69 =  5:  2c42a8f69 lib/list.c: comma_to_list(): Allow a trailing comma
 6:  fda4d2fd3 =  6:  fda4d2fd3 lib/list.c: comma_to_list(): Use xastrsep2ls() instead of its pattern
 7:  2317c26a0 =  7:  2317c26a0 lib/shadow/group/sgetgrent.c: Remove redundant code
 8:  e63a13df3 !  8:  12b367aba lib/list.c: comma_to_list(): Use build_list() instead of its pattern
    @@ lib/list.c
      
      
      /*
    +@@ lib/list.c: comma_to_list(const char *comma)
    + {
    +   char *members;
    +   char **array;
    +-  size_t  n;
    + 
    +   assert (NULL != comma);
    + 
     @@ lib/list.c: comma_to_list(const char *comma)
      
        members = xstrdup (comma);
 9:  05686fa62 =  9:  a5a569e3f lib/: Rename build_list() => acsv2ls()
10:  7dd70a10c = 10:  2505a9c4d lib/, src/: Move list.c prototypes to new file list.h
v3b
  • Rebase
$ git rd 
 1:  97b9ac63c =  1:  c2c737e4e lib/shadow/group/sgetgrent.c: Move free(3) call outside of helper
 2:  999b32c17 =  2:  c368da517 lib/shadow/gshadow/sgetsgent.c: Don't exit from library code
 3:  85f64c986 =  3:  6cb4a353e lib/, src/: Use more readable syntax for empty loops
 4:  5e0eaa277 =  4:  578b59a75 lib/: Deduplicate code
 5:  2c42a8f69 =  5:  b671f1a46 lib/list.c: comma_to_list(): Allow a trailing comma
 6:  fda4d2fd3 =  6:  8b86ea0f0 lib/list.c: comma_to_list(): Use xastrsep2ls() instead of its pattern
 7:  2317c26a0 =  7:  095be028f lib/shadow/group/sgetgrent.c: Remove redundant code
 8:  12b367aba =  8:  2d70caf13 lib/list.c: comma_to_list(): Use build_list() instead of its pattern
 9:  a5a569e3f =  9:  33b16470b lib/: Rename build_list() => acsv2ls()
10:  2505a9c4d = 10:  09688365c lib/, src/: Move list.c prototypes to new file list.h
v3c
  • Rebase
$ git rd 
 1:  c2c737e4e =  1:  93eb0220f lib/shadow/group/sgetgrent.c: Move free(3) call outside of helper
 2:  c368da517 =  2:  7fe566213 lib/shadow/gshadow/sgetsgent.c: Don't exit from library code
 3:  6cb4a353e !  3:  2831c4297 lib/, src/: Use more readable syntax for empty loops
    @@ lib/groupmem.c
     +          continue;
      
        /*@-mustfreeonly@*/
    -   gr->gr_mem = MALLOC(i + 1, char *);
    +   gr->gr_mem = malloc_T(i + 1, char *);
     
      ## lib/list.c ##
     @@ lib/list.c: dup_list(char *const *list)
    @@ lib/list.c: dup_list(char *const *list)
     +  for (i = 0; NULL != list[i]; i++)
     +          continue;
      
    -   tmp = XMALLOC(i + 1, char *);
    +   tmp = xmalloc_T(i + 1, char *);
      
     
      ## lib/sgroupio.c ##
    @@ lib/sgroupio.c
     +  for (i = 0; NULL != sgent->sg_adm[i]; i++)
     +          continue;
        /*@-mustfreeonly@*/
    -   sg->sg_adm = MALLOC(i + 1, char *);
    +   sg->sg_adm = malloc_T(i + 1, char *);
        /*@=mustfreeonly@*/
     @@
        }
    @@ lib/sgroupio.c
     +  for (i = 0; NULL != sgent->sg_mem[i]; i++)
     +          continue;
        /*@-mustfreeonly@*/
    -   sg->sg_mem = MALLOC(i + 1, char *);
    +   sg->sg_mem = malloc_T(i + 1, char *);
        /*@=mustfreeonly@*/
     
      ## src/groupdel.c ##
 4:  578b59a75 =  4:  35befcc1c lib/: Deduplicate code
 5:  b671f1a46 !  5:  ceb4fa62d lib/list.c: comma_to_list(): Allow a trailing comma
    @@ Commit message
      ## lib/list.c ##
     @@ lib/list.c: comma_to_list(const char *comma)
        n = strchrcnt(members, ',') + 2;
    -   array = XMALLOC(n, char *);
    +   array = xmalloc_T(n, char *);
      
     -  /*
     -   * Empty list is special - 0 members, not 1 empty member.  --marekm
 6:  8b86ea0f0 !  6:  3726f1aef lib/list.c: comma_to_list(): Use xastrsep2ls() instead of its pattern
    @@ lib/list.c: comma_to_list(const char *comma)
     -   */
     -
     -  n = strchrcnt(members, ',') + 2;
    --  array = XMALLOC(n, char *);
    +-  array = xmalloc_T(n, char *);
     -
     -  strsep2ls(members, ",", n, array);
     +  array = xastrsep2ls(members, ",", &n);
 7:  095be028f =  7:  dcc2f6f18 lib/shadow/group/sgetgrent.c: Remove redundant code
 8:  2d70caf13 =  8:  f5b455110 lib/list.c: comma_to_list(): Use build_list() instead of its pattern
 9:  33b16470b =  9:  e2e8660c5 lib/: Rename build_list() => acsv2ls()
10:  09688365c = 10:  df8d07178 lib/, src/: Move list.c prototypes to new file list.h
v3d
  • Rebase
$ git range-diff gh/static..gh/ls static..ls 
 1:  7fe566213 =  1:  ac39d279c lib/shadow/gshadow/sgetsgent.c: Don't exit from library code
 2:  2831c4297 =  2:  61675a896 lib/, src/: Use more readable syntax for empty loops
 3:  35befcc1c =  3:  6080b62c6 lib/: Deduplicate code
 4:  ceb4fa62d =  4:  0e889365a lib/list.c: comma_to_list(): Allow a trailing comma
 5:  3726f1aef =  5:  b24ff7962 lib/list.c: comma_to_list(): Use xastrsep2ls() instead of its pattern
 6:  dcc2f6f18 =  6:  9ce952a5e lib/shadow/group/sgetgrent.c: Remove redundant code
 7:  f5b455110 =  7:  5a638fde4 lib/list.c: comma_to_list(): Use build_list() instead of its pattern
 8:  e2e8660c5 =  8:  b13c904b0 lib/: Rename build_list() => acsv2ls()
 9:  df8d07178 =  9:  dcae3d85d lib/, src/: Move list.c prototypes to new file list.h
v3e
  • Rebase
$ git range-diff gh/static..gh/ls static..ls 
 1:  ac39d279c =  1:  2486c3de1 lib/shadow/gshadow/sgetsgent.c: Don't exit from library code
 2:  61675a896 =  2:  afe0ebf6c lib/, src/: Use more readable syntax for empty loops
 3:  6080b62c6 =  3:  70546abae lib/: Deduplicate code
 4:  0e889365a =  4:  961cc8fa4 lib/list.c: comma_to_list(): Allow a trailing comma
 5:  b24ff7962 =  5:  5e971f5e5 lib/list.c: comma_to_list(): Use xastrsep2ls() instead of its pattern
 6:  9ce952a5e =  6:  76754e2e9 lib/shadow/group/sgetgrent.c: Remove redundant code
 7:  5a638fde4 =  7:  dbe7eee33 lib/list.c: comma_to_list(): Use build_list() instead of its pattern
 8:  b13c904b0 =  8:  61b9daf51 lib/: Rename build_list() => acsv2ls()
 9:  dcae3d85d =  9:  d0c84ec6d lib/, src/: Move list.c prototypes to new file list.h

@alejandro-colomar alejandro-colomar marked this pull request as draft June 10, 2025 12:35
@alejandro-colomar alejandro-colomar force-pushed the ls branch 3 times, most recently from 08a1db1 to 866b069 Compare July 20, 2025 14:38
@alejandro-colomar alejandro-colomar force-pushed the ls branch 3 times, most recently from 6e05e52 to afca498 Compare October 2, 2025 10:42
@alejandro-colomar alejandro-colomar force-pushed the ls branch 2 times, most recently from c69ff9b to f0e13f5 Compare October 7, 2025 09:16
@alejandro-colomar alejandro-colomar marked this pull request as ready for review October 7, 2025 09:17
@alejandro-colomar alejandro-colomar force-pushed the ls branch 2 times, most recently from 4f27d71 to 5ec6add Compare October 19, 2025 06:39
@alejandro-colomar alejandro-colomar force-pushed the ls branch 2 times, most recently from fcb5151 to 989b2ab Compare November 6, 2025 12:59
@hallyn
Copy link
Member

hallyn commented Dec 6, 2025

Did you verify that the testsuites behave identically with and without this set?

@alejandro-colomar
Copy link
Collaborator Author

Did you verify that the testsuites behave identically with and without this set?

I didn't. Are you worried about any specific commit?

@hallyn
Copy link
Member

hallyn commented Dec 7, 2025

I was worried about the comma_to_list() change. The code seems correct. But it would
be good to test gpasswd with multiple admins with and without a trailing , and with an
empty list, and make sure behavior isn't changed. Like
tests/grouptools/gpasswd/72_gpasswd-M-A/gpasswd.test

@alejandro-colomar
Copy link
Collaborator Author

I was worried about the comma_to_list() change. The code seems correct. But it would be good to test gpasswd with multiple admins with and without a trailing , and with an empty list, and make sure behavior isn't changed. Like tests/grouptools/gpasswd/72_gpasswd-M-A/gpasswd.test

Okay, how about having a separate PR for that change, and then test that one thoroughly there?

I prefer doing that after the release, so we can maybe merge the first few commits before the release, and keep the other part for afterwards.

This is for consistency with how this is done in sgetsgent().

Now we need to make sure that the list is NULL before the first call,
which means that the 'grent' structure must be cleared.  Since it's
a static object, it's already true, but let's be explicit about it.

Since we remember the address of the list in grent.gr_mem (which is
static), we don't need to keep 'members' being static too, so we get rid
of one static thing.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
Imitate how this is done in sgetgrent().

BE CAREFUL:  The NULL check needs to be performed after *both*
build_list() calls, since we need to make sure that either a NULL or a
valid pointer is stored in both .sg_adm and .sg_mem, since we'll call
free(3) next time sgetsgent() is called.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Move build_list() to "lib/list.c", where we can reuse it.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
In some places we call comma_to_list(), and in others build_list(), and
they're essentially the same thing: they transform a comma-separated
list into an array of strings.

Both of them consider an empty list to have 0 members instead of one.
However, they differ in how they treat trailing commas:

-  comma_to_list() interprets that as an empty field.
-  build_list() ignores the trailing comma.

The behavior of build_list() seems more appropriate, since it allows
tools to generate the CSVs more easily.  Also, these lists are used to
represent user names or group names, so an empty user name makes no
sense.

This patch makes both functions be more consistent, which will
eventually allow us to de-duplicate code.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
get_gid() will fail to parse a number if the field is empty.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
This name tells better what this API does.

The name has a leading 'a', as it allocates memory, and then 'csv2ls'
because it parses a CSV into a list.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants