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) { _______________________________________________ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs