Re: [PATCH v5 3/9] ref: port git-fsck(1) regular refs check for files backend

From: shejialuo <shejialuo@gmail.com>
Date: Oct 9, 2024, 11:59 UTC
Message-ID: <ZwZwGAxkbQtnozh9@ArchLinux>
In-Reply-To: <ZwY5O63OI01LZX1u@pks.im>

On Wed, Oct 09, 2024 at 10:05:19AM +0200, Patrick Steinhardt wrote:

[snip]

Show 11 quoted lines

> > I may have said this before, but quite frankly, the API into the
> > fsck_report_ref() function is misdesigned.  If the single constant
> > string "cannot read ref file" cnanot give more information than
> > FSCK_MSG_BAD_REF_CONTENT, the parameter has no value.
> 
> True in the current form, yeah. If `fsck_report_ref()` learned to take a
> vararg argument and treat its first argument as a string format it would
> be justified though, as the message is now dynamic and can contain more
> context around the specific failure that cannot be provided statically
> via the 1:1 mapping between error code and message.
>

It is not "learned". At current, `fsck_report_ref` can do this and is the same as "fsck.c::report". I have explained this when replying to Junio.

Show 8 quoted lines

> > The fsck.c:report() function, which is the main function to report
> > fsck's findings before fsck_report_ref() was introduced, did not
> > have such a problem, as it allowed "const char *fmt, ..." at the
> > end.  Is it too late to fix the fsck_report_ref()?
> 
> I don't think so, I think we should be able to refactor the code rather
> easily to do so.
>

It's not hard to refactor the code. But this is not the problem. I am a little confused here. Because we already allowed "fsck_report_ref" having "const char *fmt, ..." at the end.

> Patrick

Thanks, Jialuo