Hi Paul,

On 2015-06-18 13:25, Paul Tan wrote:

> diff --git a/git-compat-util.h b/git-compat-util.h
> index 0cc7ae8..bc77d77 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -719,6 +719,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, ...);

I wonder whether it is worth it to make this a varargs function. It is not too 
much to ask callers to specify a specific mode everytime they call `xopen()`, 
no?

> diff --git a/wrapper.c b/wrapper.c
> index c1a663f..82658b3 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);
> +
> +     for (;;) {
> +             int fd = open(path, oflag, mode);
> +             if (fd >= 0)
> +                     return fd;
> +             if (errno == EINTR)
> +                     continue;
> +             die_errno(_("could not open '%s'"), path);

It is often helpful to know whether a path was opened for reading or writing, 
so maybe we should have something like

if (oflag & O_WRITE)
    die_errno(_("could not open '%s' for writing"), path);
else if (oflag & O_READ)
    die_errno(_("could not open '%s' for reading"), path);
else
    die_errno(_("could not open '%s'"), path);

? I know it is a bit of duplication, but I fear we cannot get around that 
without breaking i18n support.

Ciao,
Dscho


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to