On Tue, Feb 25, 2025 at 09:44:12AM -0800, Junio C Hamano wrote:
> shejialuo <shejialuo@xxxxxxxxx> writes:
>
> > Although "git-fsck(1)" and "packed-backend.c" will check some
> > consistency and correctness of "packed-refs" file, they never check the
> > filetype of the "packed-refs". Let's verify that the "packed-refs" has
> > the expected filetype, confirming it is created by "git pack-refs"
> > command.
> >
> > Use "lstat" to check the file mode. If we cannot check the file status
> > due to there is no such file this is OK because there is a possibility
> > that there is no "packed-refs" in the repo.
>
> Can this be done _after_ the open_nofollow() check you had in the
> previous round noticed a problem? Even though we are trying to
> notice and find problems in the given repository, it is generally
> a good idea to optimize for the more common case (i.e. the file is a
> regular one and not a symbolic link or directory or anything funny).
> Something along the lines of
>
> fd = open_nofollow(...);
> if (fd < 0) {
> lstat() to inspect the details
> } else if (fstat(fd, &st) < 0) {
> ... cannot tell what we opened ...
> } else if (!S_ISREG(st.st_mode)) {
> ... we opened something funny ...
> } else {
> ... the thing is a regular file as expected ...
> }
>
Good idea, by using this way, the code would be more clean. I will
improve this in the next version.
> perhaps?
