Re: [PATCH v6 3/9] packed-backend: check whether the "packed-refs" is regular file

[shejialuo <shejialuo@gmail.com>]

On Tue, Feb 25, 2025 at 09:44:12AM -0800, Junio C Hamano wrote:
> shejialuo <shejialuo@gmail.com> 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?