Teng Long <dyroneteng@xxxxxxxxx> writes:
> When finished call git_open(), instead of ignoring ENOENT silently
> and return error_errno(_("cannot stat...")), it's better to check
> the ENOENT before then output the warning.
Both the title and the body describes a complete opposite of what
the patch does, doesn't it?
pack-bitmap.c: do not ignore error when opening a bitmap file
Calls to git_open() to open the pack bitmap file and
multi-pack bitmap file do not report any error when they
fail. These files are optional and it is not an error if
open failed due to ENOENT, but we shouldn't be ignoring
other kinds of errors.
or something?
> + if (fd < 0) {
> + if (errno != ENOENT)
> + warning("'%s' cannot open '%s'", strerror(errno), bitmap_name);
> + free(bitmap_name);
Showing the errno is good, but I do not think it should be enclosed
in single quotes.
One thing you should consider when writing a "this is an optional
file and not finding it is perfectly fine" logic like this one is
that you may or may not want to ignore ENOTDIR. ENOTDIR is what you
get when you say open("a/b") and "a" exists as something other than
a directory. If you have a path with a slash in bitmap_name, and if
in a sane and healthy repository the leading path should always
exist (i.e. the file "a/b" may be missing, but the directory "a/"
should be there), then getting ENOTDIR is a noteworthy event. There
may be cases where ENOTDIR can justifiably and should be ignored.
