On 01/11/2011 09:05 AM, Jim Meyering wrote: >> BTW do you agree with merging my fist two patch? > > I haven't looked carefully, but this use of sprintf will > dereference NULL when malloc fails: > > - char *result = (bufsize < OPENAT_BUFFER_SIZE ? buf : xmalloc > (bufsize)); > + char *result = (bufsize < OPENAT_BUFFER_SIZE ? buf : malloc (bufsize)); > sprintf (result, PROC_SELF_FD_FORMAT, fd, file);
The idea of making openat() fail with ENOMEM instead of calling xalloc_die() is reasonable to me (POSIX doesn't explicitly document ENOMEM as a typical error from openat(), but it DOES have a global statement that any standardized functions can return errno values not documented in the standard provided that the value returned is accurate to the error in question and more specific than any of the documented errors). But like Jim said, it means you now have to do NULL checks throughout the rest of the openat implementation, in order to properly fail with ENOMEM instead of crashing. If a caller really cares about detecting ENOMEM, they can call xalloc_die themselves (but more likely, if openat() fails with ENOMEM, an app that uses xmalloc will likely fail with xalloc_die shortly thereafter on the next attempted allocation, even if such allocation is preparing the error() message for the detected failure of openat()). And fixing the xmalloc/malloc problem is only half the problem, you still have to address the problem of chdir() failures before you've completely made the openat() emulation robust. I also agree with your approach of separating the two issues into separate patches. -- Eric Blake ebl...@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
