On Wed, Sep 18, 2024 at 01:19:13PM -0700, Junio C Hamano wrote: > shejialuo <shejialuo@xxxxxxxxx> writes: > > Expect that people do not read the body of the message as completing > a paragrpah the title started. I.e. ... > > > We have already introduced the checks for regular refs. There is no need > > to check the consistency of the target which the symref points to. > > Instead, we just need to check the content of the symref itself. > > ... this needs a bit of preamble, like > > We have code that check regular ref contents, but we do not yet > check contents of symbolic refs. > Thanks, I will improve this in the next version. > > A regular file is accepted as a textual symref if it begins with > > "ref:", followed by zero or more whitespaces, followed by the full > > refname, followed only by whitespace characters. We always write > > a single SP after "ref:" and a single LF after the refname, but > > third-party reimplementations of Git may have taken advantage of the > > looser syntax. Put it more specific, we accept the following contents > > of the symref: > > > > 1. "ref: refs/heads/master " > > 2. "ref: refs/heads/master \n \n" > > 3. "ref: refs/heads/master\n\n" > > > > Thus, we could reuse "refMissingNewline" and "trailingRefContent" > > FSCK_INFOs to do the same retroactive tightening as we introduce for > > regular references. > > > > But we do not allow any other trailing garbage. The followings are bad > > symref contents which will be reported as fsck error by "git-fsck(1)". > > This description needs to be updated, as it is unclear if you are > talking about errors we already detect, or if you are planning to > update fsck to notice and report these errors. > Yes, When I was writing this part, I felt a little painful to express my words. I have thought how could I express the connection between the current patch and the previous one. > > And we will remember the untrimmed length of the "referent" and call > > "strbuf_rtrim()" on "referent". Then, we will call "check_refname_format" > > to check whether the trimmed referent format is valid. If not, we will > > report to the user that the symref points to referent which has invalid > > format. If it is valid, we will compare the untrimmed length and trimmed > > length, if they are not the same, we need to warn the user there is some > > trailing garbage in the symref content. > > That is an implementation detail of what you did. But if the > implementation were buggy and did not exactly what you intended to > do, the above description gives no information to help others to fix > it up so that it works as you intended it to work, because you do > not explain it. > > So what did you want to achieve in the third step (the first being > "limit to refs/ hiararchy", the second being "no incomplete lines > allowed")? > > Third, we want to make sure that the contents of a textual > symref MUST have a single LF after the target refname and > NOTHING ELSE. > > or something. >