Re: [PATCH 19/23] read_packed_refs(): report unexpected fopen() failures

Jeff King [off-list ref] writes:

On Wed, May 17, 2017 at 02:05:42PM +0200, Michael Haggerty wrote:

quoted

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.

quoted

+	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;
	}