On Thu, Aug 22, 2024 at 08:42:02PM +0800, shejialuo wrote:
> On Thu, Aug 22, 2024 at 10:53:57AM +0200, Patrick Steinhardt wrote:
>
>
> > > + if ((space_num || newline_num) && !isspace(*p)) {
> > > + ret = fsck_report_ref(o, report,
> > > + FSCK_MSG_BAD_REF_CONTENT,
> > > + "contains non-null garbage");
> > > + goto out;
> > > + }
> > > +
> > > + if (*p == '\n') {
> > > + newline_num++;
> > > + } else if (*p == ' ') {
> > > + space_num++;
> > > + }
> > > + p++;
> > > + }
> >
> > Can't we replace this with a single `strchr('\n')` call to check for the
> > newline and then verify that the next character is a `\0`? The check for
> > spaces would then be handled by `check_refname_format()`.
> >
>
> We cannot. Think about this situation.
>
> "ref: refs/heads/master \n "
>
> We find that the next character of '\n' is not '\0'. Then we leave it to
> "check_refname_format". But "check_refname_format" will report an error
> here, but this is an allowed symref.
Wouldn't it be correct to warn about this? To me the above very much
looks like garbage after the refname, same like we'd also warn about
such garbage for direct refs.
Patrick
