Re: [PATCH v3 3/4] ref: add symref content check for files backend

> > In order to provide above checks, we will first check whether the symref
> > content misses the newline by peeking the last byte of the "referent" to
> > see whether it is '\n'.
> 
> I'd still argue that we should do the same retroactive tightening as we
> introduce for normal references, also with an INFO level at first.
> Otherwise we're being inconsistent across the ref types.
> 

> > diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt
> > index 06d045ac48..beb6c4e49e 100644
> > --- a/Documentation/fsck-msgids.txt
> > +++ b/Documentation/fsck-msgids.txt
> > @@ -28,6 +28,10 @@
> >  `badRefName`::
> >  	(ERROR) A ref has an invalid format.
> >  
> > +`badSymrefTarget`::
> > +	(ERROR) The symref target points outside the ref directory or
> > +	the name of the symref target is invalid.
> 
> These are two separate error cases, and we even have different code
> paths raising them. Shouldn't we thus also have two different diagnostic
> codes for this?
> 

> > +	if (!skip_prefix(referent->buf, "refs/", &p)) {
> > +
> 
> There's a superfluous newline here.
> 
> Also, you never use the value of `p`, so you can instead use
> `starts_with()`.
> 

> > +	/*
> > +	 * Missing target should not be treated as any error worthy event and
> > +	 * not even warn. It is a common case that a symbolic ref points to a
> > +	 * ref that does not exist yet. If the target ref does not exist, just
> > +	 * skip the check for the file type.
> > +	 */
> > +	if (lstat(referent_path->buf, &st))
> > +		goto out;
> 
> We may also want to verify that `errno == ENOENT` here.
>