Re: [Bug-gnulib] extended xmalloc.c

[Bruno Haible]

Paul Eggert wrote:
> So, how about the patch enclosed below instead?  It attacks the
> problem systematically, by making the change at the
> malloc/free/etc. level.

Looks good to me, except for four things:

1)
> +/* Programs can define BLOCK_INPUT and UNBLOCK_INPUT in blockinput.h
> +   to enter and exit critical sections around potentially
> +   non-reentrant code like malloc.  */
The comment should also say that programs that do this must invoke

  AC_LIBOBJ(malloc)
  AC_DEFINE([malloc], [rpl_malloc])
  AC_LIBOBJ(realloc)
  AC_DEFINE([realloc], [rpl_realloc])
  AC_LIBOBJ(calloc)
  AC_DEFINE([calloc], [rpl_calloc])
  AC_LIBOBJ(free)
  AC_DEFINE([free], [rpl_free])

explicitly. Alternatively, we create a module 'interruptsafe-malloc'
whose .m4 macro contains this stuff and which depends on 'malloc', 'realloc',
'calloc', 'free'.

2) Can you choose more generic names for the macros? Like
   START_MALLOC_INVOCATION, END_MALLOC_INVOCATION
   instead of BLOCK_INPUT, UNBLOCK_INPUT. So that other programs
   can define them as they see fit. And Emacs can just
     #define START_MALLOC_INVOCATION BLOCK_INPUT
     #define END_MALLOC_INVOCATION UNBLOCK_INPUT

3)
+#ifdef DO_BLOCK_INPUT
+# include "blockinput.h"
+#else

This is too much emacsism for my taste; it makes the solution less
generic. I propose to just include <config.h>, and let Emacs' config.h
#include all that is needed.

3)
> +  if (SIZE_MAX / s < n)
> +    return NULL;

Divisions are often more expensive than other arithmetic operations, and
often calloc is called with n==1 or s==1. So it's probably a win in terms
of speed to write

    if (n > 1 && s > 1 && SIZE_MAX / s < n)
      return NULL;


Bruno