Re: [PATCH 00/15] Handle fopen() errors
- From
Duy Nguyen <pclouds@gmail.com>
- Date
- Apr 21, 2017, 11:04 UTC
- Message-ID
- <CACsJy8D1LuH6qVp15MSkCM_oQphVUUK0r9SeKC5AzX+9Xi2dcw@mail.gmail.com>
- In-Reply-To
- <20170421062915.he5tlgjqq7kj5h32@sigill.intra.peff.net>
On Fri, Apr 21, 2017 at 1:29 PM, Jeff King <peff@peff.net> wrote:
Show 30 quoted lines
> On Thu, Apr 20, 2017 at 08:41:32PM -0700, Junio C Hamano wrote:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>> > I wonder if it is OK to only special case ENOENT for !fp cases,
>> > where existing code silently returns. Perhaps it is trying to read
>> > an optional file, and it returns silently because lack of it is
>> > perfectly OK for the purpose of the code. Are there cases where
>> > this optional file is inside an optional directory, leading to
>> > ENOTDIR, instead of ENOENT, observed and reported by your check?
>>
>> "git grep -B1 warn_on_inaccessible" is enlightening. I wonder if we
>> want to wrap the two lines into a hard to misuse helper function,
>> something along this line. Would having this patch as a preparatory
>> step shrink your series? The patch count would be the same, but you
>> wouldn't be writing "if (errno != ENOENT)" lines yourself.
>
> I had a similar thought while reading through it. I think it would be
> shorter still with:
>
> FILE *fopen_or_warn(const char *path, const char *mode)
> {
> FILE *fh = fopen(path, mode);
> if (!fh)
> warn_failure_to_read_open_optional_path(path);
> return fh;
> }
>
> And then quite a few of the patches could just be
> s/fopen/fopen_or_warn/.Jeff.. oh Jeff.. you have made it _way_ too convenient that after a quick grep at fopen( again, I found a couple more places that I would have just ignored last time (too much work), but now all I need to do is Alt-f to the end of fopen and Alt-/ a few times. Too tempting.. :)
-- Duy