Re: [PATCH] Add 'safe-alloc' module

[Bruno Haible]

Hi David,

> Attached is an incremental patch with those changes.

Thanks. With the confirmation by Karl that the copyright is assigned to the FSF
both for Dan Berrange and for you, I've added your module, in two commits:


2009-02-21  David Lutterkort  <address@hidden>

        Tests for module 'safe-alloc'.
        * tests/test-safe-alloc.c: New file.
        * modules/safe-alloc-tests: New file.

        New module 'safe-alloc'.
        * lib/safe-alloc.h: New file.
        * lib/safe-alloc.c: New file.
        * m4/safe-alloc.m4: New file.
        * modules/safe-alloc: New file.
        * doc/safe-alloc.texi: New file.
        * doc/gnulib.texi: Include it.
        * MODULES.html.sh (Memory management functions <stdlib.h>): Add
        safe-alloc.


Reviewing this in detail, I propose a bit of cosmetics:
  - In the doc: Include the new section near the beginning; the end of that
    chapter deals with build infrastructure. Also use two spaces after a
    sentence terminator. When you terminate a sentence on one line and start
    a new one in the next line, 'makeinfo' inserts two spaces after the
    period. For consistency, it's best to do the same also for all other
    sentence breaks.
  - I did not understand the sentence "use calloc in favor of malloc", so
    I reworded that.
  - In safe-alloc.h and the tests: In gnulib we have the habit of including
    the header containing the specification first, right after config.h.
    This helps verifying that the header is self-contained, i.e. that it
    includes <stddef.h>, <stdlib.h> or whatever is necessary to define the
    types that it needs.

OK to commit that?


2009-02-21  Bruno Haible  <address@hidden>

        * doc/gnulib.texi: Include safe-alloc.texi earlier.
        * doc/safe-alloc.texi: Terminate sentences with a period. Use two
        spaces after a period. Put a space between a macro name and its
        argument list. Trivial rewordings.
        * lib/safe-alloc.c: Include safe-alloc.h right after config.h.
        * tests/test-safe-alloc.c: Likewise. Include stdlib.h.

--- doc/gnulib.texi.orig        2009-02-21 11:41:40.000000000 +0100
+++ doc/gnulib.texi     2009-02-21 11:13:23.000000000 +0100
@@ -5814,6 +5814,7 @@
 @menu
 * alloca::
 * alloca-opt::
+* Safe Allocation Macros::
 * String Functions in C Locale::
 * Quoting::
 * error and progname::
@@ -5824,7 +5825,6 @@
 * func::
 * warnings::
 * manywarnings::
-* Safe Allocation Macros::
 @end menu
 
 @node alloca
@@ -5837,6 +5837,8 @@
 @findex alloca
 @include alloca-opt.texi
 
address@hidden safe-alloc.texi
+
 @node String Functions in C Locale
 @section Character and String Functions in C Locale
 
@@ -5915,8 +5917,6 @@
 
 @include manywarnings.texi
 
address@hidden safe-alloc.texi
-
 @node GNU Free Documentation License
 @appendix GNU Free Documentation License
 
--- doc/safe-alloc.texi.orig    2009-02-21 11:41:40.000000000 +0100
+++ doc/safe-alloc.texi 2009-02-21 11:39:53.000000000 +0100
@@ -2,8 +2,8 @@
 @section Safe Allocation Macros
 
 The standard C library malloc/realloc/calloc/free APIs are prone to a
-number of common coding errors. The @code{safe-alloc} module provides
-macros that make it easier to avoid many of them. It still uses the
+number of common coding errors.  The @code{safe-alloc} module provides
+macros that make it easier to avoid many of them.  It still uses the
 standard C allocation functions behind the scenes.
 
 Some of the memory allocation mistakes that are commonly made are
@@ -11,72 +11,72 @@
 @itemize @bullet
 @item
 passing the incorrect number of bytes to @code{malloc}, especially
-when allocationg an array
+when allocating an array,
 @item
 fail to check the return value of @code{malloc} and @code{realloc} for
-errors
+errors,
 @item
-forget to fully initialize memory just allocated with @code{malloc}
+forget to fully initialize memory just allocated with @code{malloc},
 @item
 duplicate calls to @code{free} by forgetting to set the pointer
-variable to @code{NULL}
+variable to @code{NULL},
 @item
-leaking memory in calls to @code{realloc} when that call fails
+leaking memory in calls to @code{realloc} when that call fails.
 @end itemize
 
 The @code{safe-alloc} module addresses these problems in the following way:
 
 @itemize @bullet
 @item
-Define macros that wrap around the standard C allocation
-functions. That makes it possible to use the compiler's knowledge of
+It defines macros that wrap around the standard C allocation
+functions.  That makes it possible to use the compiler's knowledge of
 the size of objects for allocation; it also allows setting pointers
-passed in as arguments when appropriate
+passed in as arguments when appropriate.
 @item
-Use return values only for a success/fail error condition flag,
-and annotate them with GCC's @code{__warn_unused_result__}
+It uses return values only for a success/failure error condition flag,
+and annotates them with GCC's @code{__warn_unused_result__} attribute.
 @item
-Use @code{calloc} in favor of @code{malloc}
+It uses @code{calloc} instead of @code{malloc}.
 @end itemize
 
 @defmac {int} ALLOC (ptr)
 @findex ALLOC
 Allocate @code{sizeof(*ptr)} bytes of memory and store the address of
-allocated memory in @code{ptr}. Fill the newly allocated memory with
+allocated memory in @code{ptr}.  Fill the newly allocated memory with
 zeros.
 
 Returns -1 on failure, 0 on success.
 @end defmac
 
address@hidden {int} ALLOC_N(ptr, count)
address@hidden {int} ALLOC_N (ptr, count)
 @findex ALLOC_N
 Allocate an array of @code{count} elements, each @code{sizeof(*ptr)}
-bytes long and store the address of allocated memory in
address@hidden Fill the newly allocated memory with zeros.
+bytes long, and store the address of allocated memory in
address@hidden  Fill the newly allocated memory with zeros.
 
 Returns -1 on failure, 0 on success.
 @end defmac
 
address@hidden {int} ALLOC_N_UNINITIALIZED(ptr, count)
address@hidden {int} ALLOC_N_UNINITIALIZED (ptr, count)
 @findex ALLOC_N_UNINITIALIZED
 Allocate an array of @code{count} elements, each @code{sizeof(*ptr)}
-bytes long and store the address of allocated memory in
address@hidden The allocated memory is not initialized.
+bytes long, and store the address of allocated memory in
address@hidden  The allocated memory is not initialized.
 
 Returns -1 on failure, 0 on success.
 @end defmac
 
address@hidden {int} REALLOC_N(ptr, count)
address@hidden {int} REALLOC_N (ptr, count)
 @findex REALLOC_N
-Reallocate the memory pointedto by @code{ptr} to be big enough to hold
-at least @code{count} elements, each @code{sizeof(*ptr)} bytes long
-and store the address of allocated memory in @code{ptr}. If
-reallocation fails, the @code{ptr} is not modified.
+Reallocate the memory pointed to by @code{ptr} to be big enough to hold
+at least @code{count} elements, each @code{sizeof(*ptr)} bytes long,
+and store the address of allocated memory in @code{ptr}.  If
+reallocation fails, the @code{ptr} variable is not modified.
 
 Returns -1 on failure, 0 on success.
 @end defmac
 
address@hidden {void} FREE(ptr)
address@hidden {void} FREE (ptr)
 @findex FREE
 Free the memory stored in @code{ptr} and set @code{ptr} to
 @code{NULL}.
--- lib/safe-alloc.c.orig       2009-02-21 11:41:40.000000000 +0100
+++ lib/safe-alloc.c    2009-02-21 11:16:54.000000000 +0100
@@ -23,12 +23,13 @@
 
 #include <config.h>
 
+/* Specification.  */
+#include "safe-alloc.h"
+
 #include <stdlib.h>
 #include <stddef.h>
 #include <errno.h>
 
-#include "safe-alloc.h"
-
 
 /* Return 1 if an array of N objects, each of size S, cannot exist due
    to size arithmetic overflow.  S must be positive and N must be
--- lib/safe-alloc.h.orig       2009-02-21 11:41:40.000000000 +0100
+++ lib/safe-alloc.h    2009-02-21 11:15:14.000000000 +0100
@@ -1,5 +1,5 @@
 /*
- * memory.c: safer memory allocation
+ * safe-alloc.h: safer memory allocation
  *
  * Copyright (C) 2009 Free Software Foundation, Inc.
  *
@@ -108,7 +108,7 @@
 # define FREE(ptr)                              \
   do                                            \
     {                                           \
-      free(ptr);                                \
+      free (ptr);                               \
       (ptr) = NULL;                             \
     }                                           \
   while(0)
--- tests/test-safe-alloc.c.orig        2009-02-21 11:41:40.000000000 +0100
+++ tests/test-safe-alloc.c     2009-02-21 11:41:37.000000000 +0100
@@ -22,9 +22,10 @@
 
 #include <config.h>
 
-#include <stdio.h>
+#include "safe-alloc.h"
 
-#include <safe-alloc.h>
+#include <stdio.h>
+#include <stdlib.h>
 
 #define ASSERT(expr) \
   do                                                                        \
@@ -38,9 +39,11 @@
     }                                                                       \
   while (0)
 
-int main()
+int
+main ()
 {
-  struct tst {
+  struct tst
+  {
     int a;
     int b;
   };
@@ -48,16 +51,18 @@
   struct tst *p = NULL;
   int r;
 
-  r = ALLOC(p);
-  ASSERT(r >= 0);
+  r = ALLOC (p);
+  ASSERT (r >= 0);
 
-  ASSERT(p->a == 0 && p->b == 0);
+  ASSERT (p->a == 0 && p->b == 0);
 
   p->a = p->b = 42;
-  r = REALLOC_N(p, 5);
+  r = REALLOC_N (p, 5);
+
+  ASSERT (p[0].a == 42 && p[0].b == 42);
 
-  ASSERT(p[0].a == 42 && p[0].b == 42);
+  FREE (p);
+  ASSERT (p == NULL);
 
-  FREE(p);
-  ASSERT(p == NULL);
+  return 0;
 }