Re: [PATCH/WIP v3 01/31] wrapper: implement xopen()

On Wed, Jun 24, 2015 at 9:28 AM, Johannes Schindelin
[off-list ref] wrote:

Hi Paul,

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

quoted

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?

quoted

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.

This distinction was part of earlier series, but Torsten Boegershausen
suggested to leave
it out. [compare
http://thread.gmane.org/gmane.comp.version-control.git/270048/focus=270049]

Ciao,
Dscho