On Thu, May 18, 2017 at 01:57:45PM +0900, Junio C Hamano wrote:
> >> + if (!f) {
> >> + if (errno == ENOENT) {
> >> + /*
> >> + * This is OK; it just means that no
> >> + * "packed-refs" file has been written yet,
> >> + * which is equivalent to it being empty.
> >> + */
> >> + return packed_refs;
> >> + } else {
> >> + die("couldn't read %s: %s",
> >> + packed_refs_file, strerror(errno));
> >> + }
> >
> > I think this could be die_errno().
>
> I wonder what the endgame shape of this code should be, when it and
> nd/fopen-errors topic both graduate. We cannot use fopen_or_warn(),
> as we not just want to warn but want to die, e.g.
>
> f = fopen(...);
> if (!f) {
> if (warn_on_fopen_errors(...))
> die_erno(...);
> return packed_refs;
> }
If we go that route I think the die becomes just:
if (warn_on_fopen_errors(...))
die("unable to open packed refs");
or something. If we want a single die, perhaps the cleanest thing is to
put the logic into its own function:
static int missing_file_errno(int err)
{
return errno == ENOENT || errno == ENOTDIR;
}
...
if (!missing_file_errno(errno))
die_errno("couldn't read %s", packed_refs_file);
It feels a little silly to make such a trivial helper, but the point is
to encapsulate that logic. And I think it would be used inside
fopen_or_warn(), and possibly other places. The name might need some
work; that was the best I could come up with.
-Peff