Re: Openat without die

[Eric Blake]

On 01/11/2011 11:54 AM, Paul Eggert wrote:
> 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(-)

Missing the module dependency changes.

>  
>  /* 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;

Either we must depend on malloc-posix, or you need to force errno=ENOMEM
here rather than relying on malloc() to do it.

Other than those problems, this patch looks sane to me.

-- 
Eric Blake   address@hidden    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature