Re: [PATCH 19/23] read_packed_refs(): report unexpected fopen() failures
- From
Michael Haggerty <mhagger@alum.mit.edu>
- Date
- May 17, 2017, 15:27 UTC
- Message-ID
- <488f1c1d-2611-5665-34dd-23ed88769fbc@alum.mit.edu>
- In-Reply-To
- <20170517132839.fx6oebgqt5t65tia@sigill.intra.peff.net>
On 05/17/2017 03:28 PM, Jeff King wrote:
Show 14 quoted lines
> On Wed, May 17, 2017 at 02:05:42PM +0200, Michael Haggerty wrote: > >> The old code ignored any errors encountered when trying to fopen the >> "packed-refs" file, treating all such failures as if the file didn't >> exist. But it could be that there is some other error opening the >> file (e.g., permissions problems), and we don't want to silently >> ignore such problems. So report any failures that are not due to >> ENOENT. > > I think there are may be other "OK" errno values here, like ENOTDIR. > That's probably pretty unlikely in practice, though, as we're opening a > file at the root of the $GIT_DIR here (so somebody would had to have > racily changed our paths). It's probably fine to just err on the side of > safety.
Yikes, I think if somebody swaps $GIT_DIR out from under a working git command, the user would want to know about it. How would you possibly recover, anyway?
Show 14 quoted lines
>> + 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().Ah yes. I'll change it.
Thanks, Michael