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

From: shejialuo <[email-masked]>
Date: Wed, 9 Oct 2024 19:59:20 +0800

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

[snip]

> > 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.

> > 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