Re: [PATCH 04/10] packed-backend: add "packed-refs" header consistency check

> On Thu, Jan 16, 2025 at 02:57:37PM +0100, Patrick Steinhardt wrote:
> > On Sun, Jan 05, 2025 at 09:49:37PM +0800, shejialuo wrote:
> > > @@ -1779,7 +1867,24 @@ static int packed_fsck(struct ref_store *ref_store,
> > >  		goto cleanup;
> > >  	}
> > >  
> > > +	if (strbuf_read_file(&packed_ref_content, refs->path, 0) < 0) {
> > > +		/*
> > > +		 * Although we have checked that the file exists, there is a possibility
> > > +		 * that it has been removed between the lstat() and the read attempt by
> > > +		 * another process. In that case, we should not report an error.
> > > +		 */
> > > +		if (errno == ENOENT)
> > > +			goto cleanup;
> > 
> > Unlikely, but good to guard us against that condition regardless. It's
> > still not entirely race-free though because the file could meanwhile
> > have changed into a symlink, and we wouldn't notice now. We could fix
> > that by using open(O_NOFOLLOW), fstat the returne file descriptor and
> > then use `strbuf_read()` to slurp in the file.
> > 
> 
> Would this be too complicated for us to avoid race condition and we will
> introduce a lot of code to handle above logic. Because there is a
> possibility that when finishing reading the file content to the memory,
> the file could be changed into a symlink and we cannot notice. So, I
> wanna say we can't avoid race condition totally. It would be good if we
> avoid race, but what I am concern about here is that we would make the
> logic too complicated. So, could we make it unchanged?