Hi Stefan,
On 2015-06-24 18:59, Stefan Beller wrote:
> On Wed, Jun 24, 2015 at 9:28 AM, Johannes Schindelin
> <johannes.schindelin@xxxxxx> wrote:
>>
>> On 2015-06-18 13:25, Paul Tan wrote:
>>
>>> + 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html