bug-cvs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [bug-gnulib] valloc()?


From: Bruno Haible
Subject: Re: [bug-gnulib] valloc()?
Date: Thu, 3 Mar 2005 17:26:59 +0100
User-agent: KMail/1.5

Derek Price wrote:
>     - Rename macro gl_FUNC_MMAP to gl_FUNC_MMAP_ANON to sync
>       with new m4 file name.

Applied, thanks.

>     - Remove unnecessary fd set.

You can even optimize away the 'fd' variable entirely in this case and
make it a constant.

>     - Comment in header says pagealign_alloc() may return NULL.  It may not -
>       it will exit via error() instead.

Ah, good point. Coming to think of it:

  - The details of the error messages ("mmap to /dev/zero failed",
    "posix_memalign failed") both boil down to the same, namely
    "memory exhausted". For an end user, "memory exhausted" is more
    understandable, so let's use this.

  - Some programs want to do custom actions when an out-of-memory
    situation occurs. That's why we have xalloc_die(). So xalloc_die() should
    be called here instead of doing error(EXIT_FAILURE,0,"memory exhausted").

  - Some programs may want to have a pagealign_alloc() that returns NULL
    upon failure. Should pagealign_alloc() therefore return NULL or call
    xalloc_die()?

  - The answer is clear: We usually use an 'x' in the function name ('x'
    means 'checking') when xalloc_die() may be called. Therefore if we
    provide a function pagealign_alloc() which returns NULL upon failure
    and a function pagealign_xalloc() which calls xalloc_die() in this
    case, both needs are covered.

So I committed the appended patch.

> And one question:
>
> Why do you cast to char * here, when ret is actually a void *?  Is this
> necessary?
>
>   void *unaligned_ptr = xmalloc (size + pagesize - 1);
>   ret = (char *) unaligned_ptr
>         + ((- (unsigned long) unaligned_ptr) & (pagesize - 1));

This is necessary because 'void *' + 'integer' is not defined in ANSI C nor
ISO C99.
Only GCC interprets it the same way as 'char *' + 'integer'; all other
compilers give an error.

    Bruno


diff -c -3 -r1.1 pagealign_alloc.h
*** pagealign_alloc.h   3 Mar 2005 14:07:04 -0000       1.1
--- pagealign_alloc.h   3 Mar 2005 16:19:47 -0000
***************
*** 30,37 ****
     failed.  */
  extern void *pagealign_alloc (size_t size);
  
  /* Free a memory block.
!    PTR must be a pointer returned by pagealign_alloc.  */
  extern void pagealign_free (void *ptr);
  
  #endif /* _PAGEALIGN_ALLOC_H */
--- 30,42 ----
     failed.  */
  extern void *pagealign_alloc (size_t size);
  
+ /* Like pagealign_alloc, except it exits the program if the allocation
+    fails.  */
+ extern void *pagealign_xalloc (size_t size);
+ 
  /* Free a memory block.
!    PTR must be a non-NULL pointer returned by pagealign_alloc or
!    pagealign_xalloc.  */
  extern void pagealign_free (void *ptr);
  
  #endif /* _PAGEALIGN_ALLOC_H */
diff -c -3 -r1.1 pagealign_alloc.c
*** pagealign_alloc.c   3 Mar 2005 14:07:04 -0000       1.1
--- pagealign_alloc.c   3 Mar 2005 16:19:47 -0000
***************
*** 117,129 ****
    void *ret;
  #if HAVE_MMAP
    int flags;
-   static int fd = -1;  /* Only open /dev/zero once in order to avoid limiting
-                         the amount of memory we may allocate based on the
-                         number of open file descriptors.  */
  # ifdef HAVE_MAP_ANONYMOUS
    flags = MAP_ANONYMOUS | MAP_PRIVATE;
-   fd = -1;
  # else /* !HAVE_MAP_ANONYMOUS */
    flags = MAP_FILE | MAP_PRIVATE;
    if (fd == -1)
      {
--- 117,129 ----
    void *ret;
  #if HAVE_MMAP
    int flags;
  # ifdef HAVE_MAP_ANONYMOUS
+   const int fd = -1;
    flags = MAP_ANONYMOUS | MAP_PRIVATE;
  # else /* !HAVE_MAP_ANONYMOUS */
+   static int fd = -1;  /* Only open /dev/zero once in order to avoid limiting
+                         the amount of memory we may allocate based on the
+                         number of open file descriptors.  */
    flags = MAP_FILE | MAP_PRIVATE;
    if (fd == -1)
      {
***************
*** 134,148 ****
  # endif /* HAVE_MAP_ANONYMOUS */
    ret = mmap (NULL, size, PROT_READ | PROT_WRITE, flags, fd, 0);
    if (!ret)
!     error (EXIT_FAILURE, errno, "mmap to /dev/zero failed");
    new_memnode (ret, size);
  #elif HAVE_POSIX_MEMALIGN
    int status = posix_memalign (&ret, getpagesize (), size);
    if (status)
!     error (EXIT_FAILURE, status, "posix_memalign failed");
  #else /* !HAVE_MMAP && !HAVE_POSIX_MEMALIGN */
    size_t pagesize = getpagesize ();
!   void *unaligned_ptr = xmalloc (size + pagesize - 1);
    ret = (char *) unaligned_ptr
          + ((- (unsigned long) unaligned_ptr) & (pagesize - 1));
    new_memnode (ret, unaligned_ptr);
--- 134,150 ----
  # endif /* HAVE_MAP_ANONYMOUS */
    ret = mmap (NULL, size, PROT_READ | PROT_WRITE, flags, fd, 0);
    if (!ret)
!     return NULL;
    new_memnode (ret, size);
  #elif HAVE_POSIX_MEMALIGN
    int status = posix_memalign (&ret, getpagesize (), size);
    if (status)
!     return NULL;
  #else /* !HAVE_MMAP && !HAVE_POSIX_MEMALIGN */
    size_t pagesize = getpagesize ();
!   void *unaligned_ptr = malloc (size + pagesize - 1);
!   if (unaligned_ptr == NULL)
!     return NULL;
    ret = (char *) unaligned_ptr
          + ((- (unsigned long) unaligned_ptr) & (pagesize - 1));
    new_memnode (ret, unaligned_ptr);
***************
*** 151,156 ****
--- 153,170 ----
  }
  
  
+ void *
+ pagealign_xalloc (size_t size)
+ {
+   void *ret;
+ 
+   ret = pagealign_alloc (size);
+   if (ret == NULL)
+     xalloc_die ();
+   return ret;
+ }
+ 
+ 
  void
  pagealign_free (void *aligned_ptr)
  {





reply via email to

[Prev in Thread] Current Thread [Next in Thread]