Re: valloc()?

Derek Price <[email protected]> Thu, 03 Mar 2005 14:40:21 -0500
Newsgroups gmane.comp.lib.gnulib.bugs,gmane.comp.version-control.cvs.bugs
Organization Get CVS Support from Ximbiot <http://ximbiot.com>!
Message-ID <[email protected]> 
I've attached a patch to fix a few more nits, Bruno:

    - I've followed through what you did with fd and made the flags variable
      constant for both mmap() implementations.
    - I noticed that I assumed mmap() returns NULL on error when it actually
      returns MAP_FAILED according to POSIX.  I've corrected for this.
    - I define MAP_FAILED via mmap-anon.m4 when it isn't already defined.
      This change was necessary for old systems compiling CVS, though I have
      not yet heard specifically which symptoms from the reporter yet.
Possibly
      some old BSD/OS.
    - I set errno when posix_memalign fails before returning NULL.
    - I check for error return on munmap and exit fatally since this
shouldn't
      be possible without an error in the memnodes functions, but
perhaps this
      error should be ignored or trigger an abort() instead, then?
    - Note in the header that pagealign_alloc sets errno on failure.


I left the failed open of /dev/zero alone for now.

I had two further thoughts on the naming of mmap-anon.m4.  First, in my
original decision to name it mmap.m4, I did take into consideration that
there might be other mmap.m4 implementations but, ideally, I would hope
that their requirements could be merged into this mmap(-anon).m4, at the
least for simplicity's sake.  Second, technically what we are currently
calling gl_FUNC_MMAP_ANON calls AC_FUNC_MMAP, so it also verifies that
private, fixed maps to files work as well, not just anonymous maps.  It
then sets up MAP_FILE for just such uses and the code in
pagealign_alloc() makes the assumption that private, unfixed maps to
files work, based on your own statement.


2005-03-03

    * lib/pagealign_alloc.h: Note pagealign_alloc() sets errno on failure.
    * lib/pagealign_alloc.c (pagealign_alloc): Set var to const as possible.
    Check for MAP_FAILED return from mmap.  Set errno on failure of
    posix_memalign().  Catch munmap failure.
    * m4/mmap-anon.m4 (gl_FUNC_MMAP_ANON): Define
    MAP_FAILED when necessary.


Regards,

Derek

_______________________________________________
bug-gnulib mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/bug-gnulib
pagealign_alloc-nits2.diff (text/plain, 3 KB) 
Index: lib/pagealign_alloc.c
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/pagealign_alloc.c,v
retrieving revision 1.3
diff -u -p -r1.3 pagealign_alloc.c
--- lib/pagealign_alloc.c	3 Mar 2005 16:39:16 -0000	1.3
+++ lib/pagealign_alloc.c	3 Mar 2005 19:04:49 -0000
@@ -119,15 +119,14 @@ pagealign_alloc (size_t size)
 {
   void *ret;
 #if HAVE_MMAP
-  int flags;
 # ifdef HAVE_MAP_ANONYMOUS
   const int fd = -1;
-  flags = MAP_ANONYMOUS | MAP_PRIVATE;
+  const int 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;
+  const int flags = MAP_FILE | MAP_PRIVATE;
   if (fd == -1)
     {
       fd = open ("/dev/zero", O_RDONLY, 0666);
@@ -136,13 +135,16 @@ pagealign_alloc (size_t size)
     }
 # endif /* HAVE_MAP_ANONYMOUS */
   ret = mmap (NULL, size, PROT_READ | PROT_WRITE, flags, fd, 0);
-  if (!ret)
+  if (ret == MAP_FAILED)
     return NULL;
   new_memnode (ret, size);
 #elif HAVE_POSIX_MEMALIGN
   int status = posix_memalign (&ret, getpagesize (), size);
   if (status)
-    return NULL;
+    {
+      errno = status;
+      return NULL;
+    }
 #else /* !HAVE_MMAP && !HAVE_POSIX_MEMALIGN */
   size_t pagesize = getpagesize ();
   void *unaligned_ptr = malloc (size + pagesize - 1);
@@ -172,7 +174,8 @@ void
 pagealign_free (void *aligned_ptr)
 {
 #if HAVE_MMAP
-  munmap (aligned_ptr, get_memnode (aligned_ptr));
+  if (munmap (aligned_ptr, get_memnode (aligned_ptr)) < 0)
+    error (EXIT_FAILURE, errno, "Failed to unmap memory");
 #elif HAVE_POSIX_MEMALIGN
   free (aligned_ptr);
 #else
Index: lib/pagealign_alloc.h
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/pagealign_alloc.h,v
retrieving revision 1.2
diff -u -p -r1.2 pagealign_alloc.h
--- lib/pagealign_alloc.h	3 Mar 2005 16:21:00 -0000	1.2
+++ lib/pagealign_alloc.h	3 Mar 2005 19:04:49 -0000
@@ -27,7 +27,7 @@
    If SIZE is not a multiple of the system page size, it will be rounded up
    to the next multiple.
    Return a pointer to the start of the memory block, or NULL if the allocation
-   failed.  */
+   failed.  errno will be set on failure.  */
 extern void *pagealign_alloc (size_t size);

 /* Like pagealign_alloc, except it exits the program if the allocation
Index: m4/mmap-anon.m4
===================================================================
RCS file: /cvsroot/gnulib/gnulib/m4/mmap-anon.m4,v
retrieving revision 1.2
diff -u -p -r1.2 mmap-anon.m4
--- m4/mmap-anon.m4	3 Mar 2005 16:06:03 -0000	1.2
+++ m4/mmap-anon.m4	3 Mar 2005 19:04:49 -0000
@@ -49,6 +49,11 @@ AC_DEFUN([gl_FUNC_MMAP_ANON],
 [/* Define MAP_FILE when it isn't otherwise.  */
 #ifndef MAP_FILE
 # define MAP_FILE 0
+#endif
+
+/* Define MAP_FAILED for old systems which neglect to.  */
+#ifndef MAP_FAILED
+# define MAP_FAILED (void *)-1
 #endif])
   fi
 ])
signature.asc (application/pgp-signature, 253 B) 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.0 (Cygwin)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFCJ2gmLD1OTBfyMaQRAuNYAKD3PDZKwZWzUXg9DMiTGnurHYRpxgCfSB1A
JI/+XhE5byT7W90xvQI4Tis=
=rNWG
-----END PGP SIGNATURE-----