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

Hi Stefan,

On 2015-06-24 18:59, Stefan Beller wrote:

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

quoted

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

quoted

+             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]

So sorry that I missed that (it is still somewhere in my ever-growing inbox). I would have politely disagreed with Torsten if I had not missed it, though.

IMO the varargs make the code more cumbersome to read (and even fragile, because you can easily call `xopen(path, O_WRITE | O_CREATE)` and would not even get so much as a compiler warning!) and the error message does carry value: it helps you resolve the issue (it is completely unnecessary to check write permissions of the directory when a file could not be opened for reading, for example, yet if the error message does not say that and you suspect that the file could not be opened for *writing* that is exactly what you would waste your time checking).

Ciao,
Dscho