Hi Junio
On 07/02/2024 17:12, Junio C Hamano wrote:
"Phillip Wood via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
I think the helper you picked is the most sensible one, modulo a few
nits.
- We would want to teach refname_is_safe() to honor is_pseudoref()
from Karthik's series to make rules more consistent.
Yes, I held off sending this series waiting for a while but then got
impatient. We may want to split out a helper from is_pseudoref() that
just checks the name without trying to read the ref for callers like
this which are going to read the ref anyway.
- The refname_is_safe() helper is not just about the stuff at the
root level. While starts_with("refs/") is overly lenient, it
rejects nonsense like "refs/../trash". We would want to lose
"starts_with() ||" part of the condition from here.
I left the "starts_with()" in as we check the refname when we look it up
with read_ref() so it seemed like wasted effort to do it here as well.
These are minor non-blocking nits that we should keep in mind only
for longer term maintenance, #leftoverbits after the dust settles.
>
Will queue.
Thanks
Phillip
