lib/list.c: realloc(3) and memmove(3) to prevent memory leaks #1423

alejandro-colomar · 2025-12-14T12:48:31Z

Revisions:

v2

Improve commit message.
Also avoid memory leaks in del_list().

$ git rd 
1:  378deaa4b ! 1:  7ba6b1f82 lib/list.c: realloc(3) to prevent memory leaks
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>
     
      ## Commit message ##
    -    lib/list.c: realloc(3) to prevent memory leaks
    +    lib/list.c: add_list(): realloc(3) to prevent memory leaks
     
         Each call to add_list() that resulted in adding a group to the list was
         leaking the old list.  There's no need to allocate a fresh list.
-:  --------- > 2:  55b3d70d9 lib/list.c: del_list(): Reimplement with memmove(3) and realloc(3) to avoid memory leaks

v2b

Clarify assumption.

$ git rd 
1:  7ba6b1f82 = 1:  7ba6b1f82 lib/list.c: add_list(): realloc(3) to prevent memory leaks
2:  55b3d70d9 ! 2:  c4c02cc30 lib/list.c: del_list(): Reimplement with memmove(3) and realloc(3) to avoid memory leaks
    @@ Commit message
         pointer.  We already return the original pointer in some cases, so this
         must be okay.
     
    +    This change assumes that list members are unique.  As far as I can see,
    +    they are.  add_list() makes sure this is true.  If a list was manually
    +    constructed without add_list(), and uniqueness wouldn't be preserved,
    +    that would be a problem.
    +
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## lib/list.c ##

v3

del_list(): Return early, to avoid a no-op realloc(3) call. It also reduces indentation.

$ git rd 
1:  7ba6b1f82 = 1:  7ba6b1f82 lib/list.c: add_list(): realloc(3) to prevent memory leaks
2:  c4c02cc30 ! 2:  ba4c3dcb8 lib/list.c: del_list(): Reimplement with memmove(3) and realloc(3) to avoid memory leaks
    @@ lib/list.c: add_list(/*@returned@*/ /*@only@*/char **list, const char *member)
     -          if (!streq(list[i], member)) {
     -                  j++;
     -          }
    -+  if (m != n) {
    -+          memmove(&list[m], &list[m+1], n-m-1);
    -+          list[n-1] = NULL;
    -   }
    - 
    --  if (j == i) {
    --          return list;
     -  }
     -
    +-  if (j == i) {
    ++  if (m == n)
    +           return list;
    +-  }
    + 
     -  /*
     -   * Allocate a new list pointer large enough to hold all the
     -   * old entries.
    @@ lib/list.c: add_list(/*@returned@*/ /*@only@*/char **list, const char *member)
     -  }
     -
     -  tmp[j] = NULL;
    ++  memmove(&list[m], &list[m+1], n-m-1);
    ++  list[n-1] = NULL;
     +  list = xrealloc_T(list, n, char *);
      
        return tmp;

v3b

Remove blank line.

interdiff

$ git diff gh/leakls 
diff --git c/lib/list.c w/lib/list.c
index 653ccbb80..fca00cf98 100644
--- c/lib/list.c
+++ w/lib/list.c
@@ -66,7 +66,6 @@ del_list(/*@returned@*/ /*@only@*/char **list, const char *member)
                continue;
        for (m = 0; list[m] != NULL && !streq(list[m], member); m++)
                continue;
-
        if (m == n)
                return list;

range-diff

$ git rd
1:  7ba6b1f82 = 1:  7ba6b1f82 lib/list.c: add_list(): realloc(3) to prevent memory leaks
2:  ba4c3dcb8 ! 2:  16e9556be lib/list.c: del_list(): Reimplement with memmove(3) and realloc(3) to avoid memory leaks
    @@ lib/list.c: add_list(/*@returned@*/ /*@only@*/char **list, const char *member)
     -   * Scan the list for the old name.  Return the original list
     -   * pointer if it is not present.
     -   */
    -+  for (n = 0; list[n] != NULL; n++)
    -+          continue;
    -+  for (m = 0; list[m] != NULL && !streq(list[m], member); m++)
    -+          continue;
    - 
    +-
     -  for (i = j = 0; list[i] != NULL; i++) {
     -          if (!streq(list[i], member)) {
     -                  j++;
    @@ lib/list.c: add_list(/*@returned@*/ /*@only@*/char **list, const char *member)
     -  }
     -
     -  if (j == i) {
    ++  for (n = 0; list[n] != NULL; n++)
    ++          continue;
    ++  for (m = 0; list[m] != NULL && !streq(list[m], member); m++)
    ++          continue;
     +  if (m == n)
                return list;
     -  }

v3c

Fix typo.

interdiff

$ git diff 16e9556be
diff --git c/lib/list.c w/lib/list.c
index fca00cf98..ff2a2509d 100644
--- c/lib/list.c
+++ w/lib/list.c
@@ -73,7 +73,7 @@ del_list(/*@returned@*/ /*@only@*/char **list, const char *member)
        list[n-1] = NULL;
        list = xrealloc_T(list, n, char *);
 
-       return tmp;
+       return list;
 }
 
 /*

range-diff

$ git rd 
1:  7ba6b1f82 = 1:  7ba6b1f82 lib/list.c: add_list(): realloc(3) to prevent memory leaks
2:  16e9556be ! 2:  bb1baf6b2 lib/list.c: del_list(): Reimplement with memmove(3) and realloc(3) to avoid memory leaks
    @@ lib/list.c: add_list(/*@returned@*/ /*@only@*/char **list, const char *member)
     -   * Allocate a new list pointer large enough to hold all the
     -   * old entries.
     -   */
    --
    ++  memmove(&list[m], &list[m+1], n-m-1);
    ++  list[n-1] = NULL;
    ++  list = xrealloc_T(list, n, char *);
    + 
     -  tmp = xmalloc_T(j + 1, char *);
     -
     -  /*
    @@ lib/list.c: add_list(/*@returned@*/ /*@only@*/char **list, const char *member)
     -  }
     -
     -  tmp[j] = NULL;
    -+  memmove(&list[m], &list[m+1], n-m-1);
    -+  list[n-1] = NULL;
    -+  list = xrealloc_T(list, n, char *);
    - 
    -   return tmp;
    +-
    +-  return tmp;
    ++  return list;
      }
    + 
    + /*

v3d

Compact return statement.

interdiff

$ git diff bb1baf6b2
diff --git c/lib/list.c w/lib/list.c
index ff2a2509d..334baa260 100644
--- c/lib/list.c
+++ w/lib/list.c
@@ -71,9 +71,7 @@ del_list(/*@returned@*/ /*@only@*/char **list, const char *member)
 
        memmove(&list[m], &list[m+1], n-m-1);
        list[n-1] = NULL;
-       list = xrealloc_T(list, n, char *);
-
-       return list;
+       return xrealloc_T(list, n, char *);
 }
 
 /*

range-diff

$ git rd 
1:  7ba6b1f82 = 1:  7ba6b1f82 lib/list.c: add_list(): realloc(3) to prevent memory leaks
2:  bb1baf6b2 ! 2:  1e0fdab10 lib/list.c: del_list(): Reimplement with memmove(3) and realloc(3) to avoid memory leaks
    @@ lib/list.c: add_list(/*@returned@*/ /*@only@*/char **list, const char *member)
     -   * Allocate a new list pointer large enough to hold all the
     -   * old entries.
     -   */
    -+  memmove(&list[m], &list[m+1], n-m-1);
    -+  list[n-1] = NULL;
    -+  list = xrealloc_T(list, n, char *);
    - 
    +-
     -  tmp = xmalloc_T(j + 1, char *);
     -
     -  /*
    @@ lib/list.c: add_list(/*@returned@*/ /*@only@*/char **list, const char *member)
     -  tmp[j] = NULL;
     -
     -  return tmp;
    -+  return list;
    ++  memmove(&list[m], &list[m+1], n-m-1);
    ++  list[n-1] = NULL;
    ++  return xrealloc_T(list, n, char *);
      }
      
      /*

v4

Copy the NULL delimiter with memmove(3).

$ git rd 
1:  7ba6b1f82 = 1:  7ba6b1f82 lib/list.c: add_list(): realloc(3) to prevent memory leaks
2:  1e0fdab10 ! 2:  64870eb95 lib/list.c: del_list(): Reimplement with memmove(3) and realloc(3) to avoid memory leaks
    @@ lib/list.c: add_list(/*@returned@*/ /*@only@*/char **list, const char *member)
     -  tmp[j] = NULL;
     -
     -  return tmp;
    -+  memmove(&list[m], &list[m+1], n-m-1);
    -+  list[n-1] = NULL;
    ++  memmove(&list[m], &list[m+1], n-m);
     +  return xrealloc_T(list, n, char *);
      }

v4b

Remove blank line.

$ git rd 
1:  7ba6b1f82 = 1:  7ba6b1f82 lib/list.c: add_list(): realloc(3) to prevent memory leaks
2:  64870eb95 ! 2:  6cbed79db lib/list.c: del_list(): Reimplement with memmove(3) and realloc(3) to avoid memory leaks
    @@ lib/list.c: add_list(/*@returned@*/ /*@only@*/char **list, const char *member)
     +  if (m == n)
                return list;
     -  }
    - 
    +-
     -  /*
     -   * Allocate a new list pointer large enough to hold all the
     -   * old entries.

v5

Iterate, to remove repeated entries.

interdiff

$ git diff gh/leakls 
diff --git c/lib/list.c w/lib/list.c
index ee56a7560..39a650567 100644
--- c/lib/list.c
+++ w/lib/list.c
@@ -62,13 +62,16 @@ del_list(/*@returned@*/ /*@only@*/char **list, const char *member)
        assert (NULL != member);
        assert (NULL != list);
 
-       for (n = 0; list[n] != NULL; n++)
-               continue;
-       for (m = 0; list[m] != NULL && !streq(list[m], member); m++)
-               continue;
-       if (m == n)
-               return list;
-       memmove(&list[m], &list[m+1], n-m);
+       do {
+               for (n = 0; list[n] != NULL; n++)
+                       continue;
+               for (m = 0; list[m] != NULL && !streq(list[m], member); m++)
+                       continue;
+               if (m == n)
+                       return list;
+               memmove(&list[m], &list[m+1], n-m);
+       } while (m != n);
+
        return xrealloc_T(list, n, char *);
 }

range-diff

$ git rd
1:  7ba6b1f82 = 1:  7ba6b1f82 lib/list.c: add_list(): realloc(3) to prevent memory leaks
2:  6cbed79db ! 2:  c6a7a19a8 lib/list.c: del_list(): Reimplement with memmove(3) and realloc(3) to avoid memory leaks
    @@ Commit message
         pointer.  We already return the original pointer in some cases, so this
         must be okay.
     
    -    This change assumes that list members are unique.  As far as I can see,
    -    they are.  add_list() makes sure this is true.  If a list was manually
    -    constructed without add_list(), and uniqueness wouldn't be preserved,
    -    that would be a problem.
    -
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## lib/list.c ##
    @@ lib/list.c: add_list(/*@returned@*/ /*@only@*/char **list, const char *member)
     -   * Scan the list for the old name.  Return the original list
     -   * pointer if it is not present.
     -   */
    --
    ++  do {
    ++          for (n = 0; list[n] != NULL; n++)
    ++                  continue;
    ++          for (m = 0; list[m] != NULL && !streq(list[m], member); m++)
    ++                  continue;
    ++          if (m == n)
    ++                  return list;
    ++          memmove(&list[m], &list[m+1], n-m);
    ++  } while (m != n);
    + 
     -  for (i = j = 0; list[i] != NULL; i++) {
     -          if (!streq(list[i], member)) {
     -                  j++;
    @@ lib/list.c: add_list(/*@returned@*/ /*@only@*/char **list, const char *member)
     -  }
     -
     -  if (j == i) {
    -+  for (n = 0; list[n] != NULL; n++)
    -+          continue;
    -+  for (m = 0; list[m] != NULL && !streq(list[m], member); m++)
    -+          continue;
    -+  if (m == n)
    -           return list;
    +-          return list;
     -  }
     -
     -  /*
    @@ lib/list.c: add_list(/*@returned@*/ /*@only@*/char **list, const char *member)
     -  tmp[j] = NULL;
     -
     -  return tmp;
    -+  memmove(&list[m], &list[m+1], n-m);
     +  return xrealloc_T(list, n, char *);
      }

v6

del_list(): Don't shrink the allocation. It's unnecessary, and this way we avoid an error condition. It also simplifies the implementation. The wasted memory shouldn't be important; after all, the memory isn't leaked.

$ git rd 
1:  7ba6b1f82 = 1:  7ba6b1f82 lib/list.c: add_list(): realloc(3) to prevent memory leaks
2:  c6a7a19a8 ! 2:  ee48f1a10 lib/list.c: del_list(): Reimplement with memmove(3) and realloc(3) to avoid memory leaks
    @@ lib/list.c: add_list(/*@returned@*/ /*@only@*/char **list, const char *member)
     +                  continue;
     +          for (m = 0; list[m] != NULL && !streq(list[m], member); m++)
     +                  continue;
    -+          if (m == n)
    -+                  return list;
     +          memmove(&list[m], &list[m+1], n-m);
     +  } while (m != n);
      
    @@ lib/list.c: add_list(/*@returned@*/ /*@only@*/char **list, const char *member)
     -  tmp[j] = NULL;
     -
     -  return tmp;
    -+  return xrealloc_T(list, n, char *);
    ++  return list;
      }
      
      /*

v7

Add missing sizeof() multiplication.

$ git rd 
1:  7ba6b1f82 = 1:  7ba6b1f82 lib/list.c: add_list(): realloc(3) to prevent memory leaks
2:  ee48f1a10 ! 2:  0d2aceb32 lib/list.c: del_list(): Reimplement with memmove(3) and realloc(3) to avoid memory leaks
    @@ lib/list.c: add_list(/*@returned@*/ /*@only@*/char **list, const char *member)
     +                  continue;
     +          for (m = 0; list[m] != NULL && !streq(list[m], member); m++)
     +                  continue;
    -+          memmove(&list[m], &list[m+1], n-m);
    ++          memmove(&list[m], &list[m+1], (n-m) * sizeof(char *));
     +  } while (m != n);
      
     -  for (i = j = 0; list[i] != NULL; i++) {

v8

Add memmove_T(), and use it instead of memmove(3). This adds type safety.

$ git rd 
1:  7ba6b1f82 = 1:  7ba6b1f82 lib/list.c: add_list(): realloc(3) to prevent memory leaks
2:  0d2aceb32 = 2:  0d2aceb32 lib/list.c: del_list(): Reimplement with memmove(3) and realloc(3) to avoid memory leaks
-:  --------- > 3:  c529f833e lib/string/strcpy/: memmove_T(): Add API
-:  --------- > 4:  cdf9c9c71 lib/list.c: del_list(): Use memmove_T() instead of its pattern

v9

Clarify in the commit message why I didn't accept const-qualified input.
Convert the return value to the right type.

$ git rd 
1:  7ba6b1f82 = 1:  7ba6b1f82 lib/list.c: add_list(): realloc(3) to prevent memory leaks
2:  0d2aceb32 = 2:  0d2aceb32 lib/list.c: del_list(): Reimplement with memmove(3) and realloc(3) to avoid memory leaks
3:  c529f833e ! 3:  b814a74fc lib/string/strcpy/: memmove_T(): Add API
    @@ Metadata
      ## Commit message ##
         lib/string/strcpy/: memmove_T(): Add API
     
    +    An interesting detail is that we require the second argument to be
    +    non-const, while the memmove(3) function gets a const void*.  This is
    +    because the second argument should usually be just the same as the first
    +    one, plus some offset, and thus will have the same const qualification
    +    (that is, it will not be qualified).
    +
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## lib/Makefile.am ##
    @@ lib/string/strcpy/memmove.h (new)
     +({                                                                    \
     +  _Generic(dst, T *: (void)0);                                  \
     +  _Generic(src, T *: (void)0);                                  \
    -+  memmove(dst, src, (n) * sizeof(T));                           \
    ++  (T *){memmove(dst, src, (n) * sizeof(T))};                    \
     +})
     +
     +
4:  cdf9c9c71 = 4:  79b7c3c53 lib/list.c: del_list(): Use memmove_T() instead of its pattern

v10

Reintroduce the realloc(3) call. This is safer, as sanitizers or valgrind will more easily detect bounds violations. BTW, after the loop, we must reallocate n+1, not n.

$ git rd 
1:  7ba6b1f82 = 1:  7ba6b1f82 lib/list.c: add_list(): realloc(3) to prevent memory leaks
2:  0d2aceb32 ! 2:  f9d14ed08 lib/list.c: del_list(): Reimplement with memmove(3) and realloc(3) to avoid memory leaks
    @@ lib/list.c: add_list(/*@returned@*/ /*@only@*/char **list, const char *member)
     -  tmp[j] = NULL;
     -
     -  return tmp;
    -+  return list;
    ++  return xrealloc_T(list, n+1, char *);
      }
      
      /*
3:  b814a74fc = 3:  3684af5f7 lib/string/strcpy/: memmove_T(): Add API
4:  79b7c3c53 ! 4:  c21af3c4d lib/list.c: del_list(): Use memmove_T() instead of its pattern
    @@ lib/list.c: del_list(/*@returned@*/ /*@only@*/char **list, const char *member)
     +          memmove_T(&list[m], &list[m+1], n-m, char *);
        } while (m != n);
      
    -   return list;
    +   return xrealloc_T(list, n+1, char *);

v10b

Document memmove_T() in lib/string/README.

$ git rd 
1:  7ba6b1f82 = 1:  7ba6b1f82 lib/list.c: add_list(): realloc(3) to prevent memory leaks
2:  f9d14ed08 = 2:  f9d14ed08 lib/list.c: del_list(): Reimplement with memmove(3) and realloc(3) to avoid memory leaks
3:  3684af5f7 ! 3:  65a37cc49 lib/string/strcpy/: memmove_T(): Add API
    @@ lib/Makefile.am: libshadow_la_SOURCES = \
        string/strcpy/stpecpy.h \
        string/strcpy/strncat.c \
     
    + ## lib/string/README ##
    +@@ lib/string/README: strcpy/ - String copying
    +     MEMCPY()
    +   Like memcpy(3), but takes two arrays.
    + 
    ++    memmove_T()
    ++  Like memmove(3), but type safe.
    ++
    + sprintf/ - Formatted string creation
    + 
    +     aprintf()
    +
      ## lib/string/strcpy/memmove.c (new) ##
     @@
     +// SPDX-FileCopyrightText: 2025, Alejandro Colomar <alx@kernel.org>
4:  c21af3c4d = 4:  8c95007b1 lib/list.c: del_list(): Use memmove_T() instead of its pattern

v10c

Rebase

$ git rd
1:  7ba6b1f82 = 1:  4258abf02 lib/list.c: add_list(): realloc(3) to prevent memory leaks
2:  f9d14ed08 = 2:  3818725e1 lib/list.c: del_list(): Reimplement with memmove(3) and realloc(3) to avoid memory leaks
3:  65a37cc49 = 3:  7639438e1 lib/string/strcpy/: memmove_T(): Add API
4:  8c95007b1 = 4:  5e4650e6b lib/list.c: del_list(): Use memmove_T() instead of its pattern

v10d

Rebase

$ git rd 
1:  4258abf02 = 1:  fcbd03d7e lib/list.c: add_list(): realloc(3) to prevent memory leaks
2:  3818725e1 = 2:  dc9dcb3f9 lib/list.c: del_list(): Reimplement with memmove(3) and realloc(3) to avoid memory leaks
3:  7639438e1 = 3:  acfbbd9dc lib/string/strcpy/: memmove_T(): Add API
4:  5e4650e6b = 4:  76373568d lib/list.c: del_list(): Use memmove_T() instead of its pattern

Each call to add_list() that resulted in adding a group to the list was leaking the old list. There's no need to allocate a fresh list. Otherwise, it would be incorrect to return the original pointer in some cases, which we already did. Signed-off-by: Alejandro Colomar <alx@kernel.org>

… avoid memory leaks Like del_list(), each call to del_list() that wasn't a no-op, was leaking the old list. There's no need to allocate a fresh list; we can use memmove(3) to delete the old member, and return the original pointer. We already return the original pointer in some cases, so this must be okay. Signed-off-by: Alejandro Colomar <alx@kernel.org>

An interesting detail is that we require the second argument to be non-const, while the memmove(3) function gets a const void*. This is because the second argument should usually be just the same as the first one, plus some offset, and thus will have the same const qualification (that is, it will not be qualified). Signed-off-by: Alejandro Colomar <alx@kernel.org>

This adds type safety. Signed-off-by: Alejandro Colomar <alx@kernel.org>

alejandro-colomar requested review from hallyn and ikerexxe December 14, 2025 12:48

alejandro-colomar force-pushed the leakls branch 2 times, most recently from 550cffb to 378deaa Compare December 14, 2025 13:03

alejandro-colomar self-assigned this Dec 14, 2025

alejandro-colomar force-pushed the leakls branch from 378deaa to 55b3d70 Compare December 14, 2025 13:33

alejandro-colomar changed the title ~~lib/list.c: realloc(3) to prevent memory leaks~~ lib/list.c: realloc(3) and memmove(3) to prevent memory leaks Dec 14, 2025

alejandro-colomar force-pushed the leakls branch 9 times, most recently from c6a7a19 to ee48f1a Compare December 14, 2025 14:40

alejandro-colomar marked this pull request as draft December 14, 2025 14:51

alejandro-colomar force-pushed the leakls branch from ee48f1a to 0d2aceb Compare December 14, 2025 15:11

alejandro-colomar marked this pull request as ready for review December 14, 2025 15:20

alejandro-colomar mentioned this pull request Dec 14, 2025

strmove(): Add API, and use it instead of its pattern #1424

Open

alejandro-colomar force-pushed the leakls branch 3 times, most recently from c21af3c to 8c95007 Compare December 15, 2025 00:42

alejandro-colomar force-pushed the leakls branch from 8c95007 to 5e4650e Compare December 23, 2025 22:00

alejandro-colomar added 4 commits December 31, 2025 03:24

lib/list.c: del_list(): Use memmove_T() instead of its pattern

7637356

This adds type safety. Signed-off-by: Alejandro Colomar <alx@kernel.org>

alejandro-colomar force-pushed the leakls branch from 5e4650e to 7637356 Compare December 31, 2025 02:27

alejandro-colomar added the New API label Jan 1, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

lib/list.c: realloc(3) and memmove(3) to prevent memory leaks #1423

lib/list.c: realloc(3) and memmove(3) to prevent memory leaks #1423

alejandro-colomar commented Dec 14, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

lib/list.c: realloc(3) and memmove(3) to prevent memory leaks #1423

Are you sure you want to change the base?

lib/list.c: realloc(3) and memmove(3) to prevent memory leaks #1423

Conversation

alejandro-colomar commented Dec 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

alejandro-colomar commented Dec 14, 2025 •

edited

Loading