Hi Duy, On Fri, 8 Mar 2019, Nguyễn Thái Ngọc Duy wrote: > diff --git a/refs.c b/refs.c > index 142888a40a..e9f83018f0 100644 > --- a/refs.c > +++ b/refs.c > @@ -72,30 +72,57 @@ static unsigned char refname_disposition[256] = { > * - it ends with ".lock", or > * - it contains a "@{" portion > */ > -static int check_refname_component(const char *refname, int *flags) > +static int check_refname_component(const char *refname, int *flags, > + struct strbuf *sanitized) > { > const char *cp; > char last = '\0'; > + size_t component_start; This variable is uninitialized. It is then... > + > + if (sanitized) > + component_start = sanitized->len; ... initialized only when `sanitized` is not `NULL`, and subsequently... > > for (cp = refname; ; cp++) { > int ch = *cp & 255; > unsigned char disp = refname_disposition[ch]; > + > + if (sanitized && disp != 1) > + strbuf_addch(sanitized, ch); > + > switch (disp) { > case 1: > goto out; > case 2: > - if (last == '.') > - return -1; /* Refname contains "..". */ > + if (last == '.') { /* Refname contains "..". */ > + if (sanitized) > + sanitized->len--; /* collapse ".." to single "." */ > + else > + return -1; > + } > break; > case 3: > - if (last == '@') > - return -1; /* Refname contains "@{". */ > + if (last == '@') { /* Refname contains "@{". */ > + if (sanitized) > + sanitized->buf[sanitized->len-1] = '-'; > + else > + return -1; > + } > break; > case 4: > - return -1; > + /* forbidden char */ > + if (sanitized) > + sanitized->buf[sanitized->len-1] = '-'; > + else > + return -1; > + break; > case 5: > - if (!(*flags & REFNAME_REFSPEC_PATTERN)) > - return -1; /* refspec can't be a pattern */ > + if (!(*flags & REFNAME_REFSPEC_PATTERN)) { > + /* refspec can't be a pattern */ > + if (sanitized) > + sanitized->buf[sanitized->len-1] = '-'; > + else > + return -1; > + } > > /* > * Unset the pattern flag so that we only accept > @@ -109,26 +136,48 @@ static int check_refname_component(const char *refname, int *flags) > out: > if (cp == refname) > return 0; /* Component has zero length. */ > - if (refname[0] == '.') > - return -1; /* Component starts with '.'. */ > + > + if (refname[0] == '.') { /* Component starts with '.'. */ > + if (sanitized) > + sanitized->buf[component_start] = '-'; ... used a loooooooong time after that, also only if `sanitized` is not `NULL`. Apparently for some GCC versions, this is too cute, and it complains that this variable might be used uninitialized: https://dev.azure.com/gitgitgadget/git/_build/results?buildId=4352&view=logs And quite honestly, even for mere humans it is not all *that* clear that `sanitized` cannot be changed from `NULL` to non-`NULL` in the code in between, *in particular* because the changes extend over two hunks, the code between is not shown. I would strongly advise against trying to be so cute, and just initialize the variable already. Over-optimization in such instances makes the code a lot harder to reason about. Ciao, Johannes