Re: Openat without die

[Paul Eggert]

I also don't see the need for all those checks for
empty strings and the like.  Nor do I see the need
for carefully setting errno = ENOMEM, as the calling
code will try something else if malloc fails and maybe
the something else will work and maybe it won't, but
the calling code should set errno appropriately for
whatever it tried.

I noticed that the existing code is slightly too pessimistic
about OPENAT_BUFFER_SIZE, which means an unnecessary
call to malloc in some cases.

One other thing; there should be no need to test
whether buf == NULL; the caller is supposed to pass
a nonnull buf.

So I propose the following patch instead, which I came up
with before reading Eric's nice review, but which I
think agrees with his ideas, and adds the abovementioned
tweaks.

I haven't pushed this.

>From 0c03ad4d899710d851135e1e72f1821e72fffe7e Mon Sep 17 00:00:00 2001
From: Paul Eggert <address@hidden>
Date: Tue, 11 Jan 2011 10:42:55 -0800
Subject: [PATCH] openat: avoid xmalloc

This removes a dependency on openat-die.  This change causes the
openat substitute to fall back on savedir when memory is tight,
but that's good enough.
* lib/openat-proc.c: Include stdlib.h (for malloc), not
xalloc.h (for xmalloc).
(openat_proc_name): Check for malloc failure.
---
 ChangeLog         |    8 ++++++++
 lib/openat-proc.c |   13 ++++++++++---
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index f8cd305..06ee9db 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2011-01-11  Paul Eggert  <address@hidden>
 
+       openat: avoid xmalloc
+       This removes a dependency on openat-die.  This change causes the
+       openat substitute to fall back on savedir when memory is tight,
+       but that's good enough.
+       * lib/openat-proc.c: Include stdlib.h (for malloc), not
+       xalloc.h (for xmalloc).
+       (openat_proc_name): Check for malloc failure.
+
        openat: Increase OPENAT_BUFFER_SIZE from 512 to at least 1024
        This avoids heap allocation for file names whose lengths are in
        the range 512..1023, with the upper bound increasing to at most
diff --git a/lib/openat-proc.c b/lib/openat-proc.c
index 51a8aa2..34c74a7 100644
--- a/lib/openat-proc.c
+++ b/lib/openat-proc.c
@@ -26,13 +26,13 @@
 #include <fcntl.h>
 
 #include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
 
 #include "dirname.h"
 #include "intprops.h"
 #include "same-inode.h"
-#include "xalloc.h"
 
 /* The results of open() in this file are not used with fchdir,
    and we do not leak fds to any single-threaded code that could use stdio,
@@ -52,7 +52,8 @@
 /* Set BUF to the expansion of PROC_SELF_FD_FORMAT, using FD and FILE
    respectively for %d and %s.  If successful, return BUF if the
    result fits in BUF, dynamically allocated memory otherwise.  But
-   return NULL if /proc is not reliable.  */
+   return NULL if /proc is not reliable, either because the operating
+   system support is lacking or because memory is low.  */
 char *
 openat_proc_name (char buf[OPENAT_BUFFER_SIZE], int fd, char const *file)
 {
@@ -98,7 +99,13 @@ openat_proc_name (char buf[OPENAT_BUFFER_SIZE], int fd, char 
const *file)
   else
     {
       size_t bufsize = PROC_SELF_FD_NAME_SIZE_BOUND (strlen (file));
-      char *result = (bufsize < OPENAT_BUFFER_SIZE ? buf : xmalloc (bufsize));
+      char *result = buf;
+      if (OPENAT_BUFFER_SIZE < bufsize)
+        {
+          result = malloc (bufsize);
+          if (! result)
+            return NULL;
+        }
       sprintf (result, PROC_SELF_FD_FORMAT, fd, file);
       return result;
     }
-- 
1.7.3