On Sun, Jun 28, 2015 at 9:48 PM, Torsten Bögershausen <tboegi@web.de> wrote:
Show 60 quoted lines
> On 06/28/2015 04:05 PM, Paul Tan wrote:
>>
>> A common usage pattern of open() is to check if it was successful, and
>> die() if it was not:
>>
>> int fd = open(path, O_WRONLY | O_CREAT, 0777);
>> if (fd < 0)
>> die_errno(_("Could not open '%s' for writing."), path);
>>
>> Implement a wrapper function xopen() that does the above so that we can
>> save a few lines of code, and make the die() messages consistent.
>>
>> Helped-by: Torsten Bögershausen <tboegi@web.de>
>> Helped-by: Jeff King <peff@peff.net>
>> Signed-off-by: Paul Tan <pyokagan@gmail.com>
>> ---
>> git-compat-util.h | 1 +
>> wrapper.c | 25 +++++++++++++++++++++++++
>> 2 files changed, 26 insertions(+)
>>
>> diff --git a/git-compat-util.h b/git-compat-util.h
>> index c6d391f..e168dfd 100644
>> --- a/git-compat-util.h
>> +++ b/git-compat-util.h
>> @@ -717,6 +717,7 @@ extern void *xrealloc(void *ptr, size_t size);
>> extern void *xcalloc(size_t nmemb, size_t size);
>> extern void *xmmap(void *start, size_t length, int prot, int flags, int
>> fd, off_t offset);
>> extern void *xmmap_gently(void *start, size_t length, int prot, int
>> flags, int fd, off_t offset);
>> +extern int xopen(const char *path, int flags, ...);
>> extern ssize_t xread(int fd, void *buf, size_t len);
>> extern ssize_t xwrite(int fd, const void *buf, size_t len);
>> extern ssize_t xpread(int fd, void *buf, size_t len, off_t offset);
>> diff --git a/wrapper.c b/wrapper.c
>> index ff49807..7e13ae0 100644
>> --- a/wrapper.c
>> +++ b/wrapper.c
>> @@ -189,6 +189,31 @@ void *xcalloc(size_t nmemb, size_t size)
>> # endif
>> #endif
>> +/**
>> + * xopen() is the same as open(), but it die()s if the open() fails.
>> + */
>> +int xopen(const char *path, int oflag, ...)
>> +{
>> + mode_t mode = 0;
>> + va_list ap;
>> +
>> + va_start(ap, oflag);
>> + if (oflag & O_CREAT)
>> + mode = va_arg(ap, mode_t);
>> + va_end(ap);
>> +
>> + assert(path);
>> +
>
> 2 remarks:
> - I don't know if and why we need the assert() here (but don't know if we
> have a strategie in Git for assert())Show 32 quoted lines
> - Having xopen() with 2 or 3 parameter is good, but the current may need
> some tweaks for better portability:
>
> int xopen(const char *path, int oflag, ...)
> {
> mode_t mode = 0;
> if (oflag & O_CREAT) {
> va_list ap;
> va_start(ap, oflag);
> mode = va_arg(ap, int);
> va_end(ap);
>
> }
>
> See e.g.
> <http://blitiri.com.ar/git/r/libfiu/c/37f6a98110e3bb59bbb4971241baa3a385c3f724/>
> why va_arg(ap, int) should be used:
>
>
> +
> + /* va_arg() can only take fully promoted types, and mode_t
> + * sometimes is smaller than an int, so we should always
> pass
> + * int to it, and not mode_t. Not doing so would may result
> in
> + * a compile-time warning and run-time error. We asume that
> it
> + * is never bigger than an int, which holds in practise. */
> + mode = va_arg(l, int);
> +
>
>Stefan Beller <sbeller@google.com>