Re: [Bug-gnulib] linebreak.c proposed patches for size-calculation overflows

[Paul Eggert]

Bruno Haible <address@hidden> writes:

> > And I'd omit the trailing '_p' (as I'm not a big fan of Hungarian
> > notation :-).
> 
> This isn't hungarian notation, it's the common notation in gnulib

The vast majority of predicates in gnulib do not end in '_p', and I'd
rather not add new predicates with '_p' suffixes.  The '_p' suffix is
philosphically related to Hungarian notation.  The basic idea is that
the type of an identifier should be related to (or deducible from) its
name.  I'm not a big fan of this idea in general, as it makes names
longer and less readable, and in the end it tends to detract from
readability and maintainability.  Admittedly this is a minor issue.

> The majority of the allocations in gnulib is done through xmalloc,
> which gnulib "owns", so here I don't mind whether xmalloc does the
> test against SIZE_MAX or not.

OK, I installed this patch to implement that.

This patch causes xsize.h to conflict with xalloc.h, because xalloc.h
now uses SIZE_MAX in a preprocessor context.  (Honest!  I'm not trying
to be difficult!  It was simply the easiest portable way to
optimize-away the need for the test against SIZE_MAX.)  No module or
application currently includes both include files, so it's not urgent
to fix this incompatibility now; whoever needs to include both files
can fix xsize.h so that it defines SIZE_MAX compatibly.

2003-11-06  Paul Eggert  <address@hidden>

        * xalloc.h [HAVE_STDINT_H]: Include <stdint.h>.
        (xalloc_oversized) [! (PTRDIFF_MAX < SIZE_MAX)]:
        Reject sizes of exactly SIZE_MAX bytes.
        * xreadlink.c: Include "xalloc.h" before checking whether SIZE_MAX
        is defined, since "xalloc.h" now defines SIZE_MAX on modern hosts.
        
Index: lib/xalloc.h
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/xalloc.h,v
retrieving revision 1.20
diff -p -u -r1.20 xalloc.h
--- lib/xalloc.h        30 Oct 2003 06:33:40 -0000      1.20
+++ lib/xalloc.h        6 Nov 2003 19:39:27 -0000
@@ -21,6 +21,9 @@
 # define XALLOC_H_
 
 # include <stddef.h>
+# if HAVE_STDINT_H
+#  include <stdint.h>
+# endif
 
 # ifndef __attribute__
 #  if __GNUC__ < 2 || (__GNUC__ == 2 && __GNUC_MINOR__ < 8) || __STRICT_ANSI__
@@ -62,8 +65,19 @@ char *xstrdup (const char *str);
 /* 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
    nonnegative.  This is a macro, not an inline function, so that it
-   works correctly even when SIZE_MAX < N.  */
-#define xalloc_oversized(n, s) ((size_t) -1 / (s) < (n))
+   works correctly even when SIZE_MAX < N.
+
+   By gnulib convention, SIZE_MAX represents overflow in size
+   calculations, so reject attempted allocations of exactly SIZE_MAX
+   bytes.  However, malloc (SIZE_MAX) fails on all known hosts where
+   PTRDIFF_MAX < SIZE_MAX, so do not bother to test for
+   exactly-SIZE_MAX allocations on such hosts; this avoids a test and
+   branch when S is known to be 1.  */
+# if defined PTRDIFF_MAX && PTRDIFF_MAX < SIZE_MAX
+#  define xalloc_oversized(n, s) ((size_t) -1 / (s) < (n))
+# else
+#  define xalloc_oversized(n, s) ((size_t) -1 / (s) <= (n))
+# endif
 
 /* These macros are deprecated; they will go away soon, and are retained
    temporarily only to ease conversion to the functions described above.  */
Index: lib/xreadlink.c
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/xreadlink.c,v
retrieving revision 1.11
diff -p -u -r1.11 xreadlink.c
--- lib/xreadlink.c     10 Sep 2003 14:52:49 -0000      1.11
+++ lib/xreadlink.c     6 Nov 2003 19:39:28 -0000
@@ -36,15 +36,15 @@ extern int errno;
 # include <unistd.h>
 #endif
 
+#include "xalloc.h"
+#include "xreadlink.h"
+
 #ifndef SIZE_MAX
 # define SIZE_MAX ((size_t) -1)
 #endif
 #ifndef SSIZE_MAX
 # define SSIZE_MAX ((ssize_t) (SIZE_MAX / 2))
 #endif
-
-#include "xalloc.h"
-#include "xreadlink.h"
 
 /* Call readlink to get the symbolic link value of FILENAME.
    Return a pointer to that NUL-terminated string in malloc'd storage.