| Message ID | a6a0217ce6fa2a7436724d76fc50fd6f8b925de5.1570477135.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | quote: handle null and empty strings in sq_quote_buf_pretty() | expand |
"Garima Singh via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Garima Singh <garima.singh@microsoft.com> > > Empty arguments passed on the command line can be a represented by > a '', however sq_quote_buf_pretty was incorrectly dropping these > arguments altogether. Fix this problem by ensuring that such > arguments are emitted as '' instead. > > Reported by: Junio Hamano <gitster@pobox.com> > Signed-off-by: Garima Singh <garima.singh@microsoft.com> > --- > quote.c | 9 +++++++++ > t/t0014-alias.sh | 7 +++++++ > 2 files changed, 16 insertions(+) > > diff --git a/quote.c b/quote.c > index 7f2aa6faa4..26f1848dde 100644 > --- a/quote.c > +++ b/quote.c > @@ -48,6 +48,15 @@ void sq_quote_buf_pretty(struct strbuf *dst, const char *src) > static const char ok_punct[] = "+,-./:=@_^"; > const char *p; > > + if (!src) > + BUG("Cannot append a NULL token to the buffer"); Remove these two lines. I do not want to see "if (!ptr) BUG("don't give a NULL pointer")" sprinkled to every function that takes a pointer that must not be NULL. Any caller that violates the contract with the callee deserves a segfault, so let's leave it at that. > + /* Avoid losing a zero-length string by adding '' */ > + if (!*src) { > + strbuf_addstr(dst, "''"); > + return; > + } > + Nice. > for (p = src; *p; p++) { > if (!isalpha(*p) && !isdigit(*p) && !strchr(ok_punct, *p)) { > sq_quote_buf(dst, src); > diff --git a/t/t0014-alias.sh b/t/t0014-alias.sh > index a070e645d7..2694c81afd 100755 > --- a/t/t0014-alias.sh > +++ b/t/t0014-alias.sh > @@ -37,4 +37,11 @@ test_expect_success 'looping aliases - internal execution' ' > # test_i18ngrep "^fatal: alias loop detected: expansion of" output > #' > > +test_expect_success 'run-command formats empty args properly' ' > + GIT_TRACE=1 git frotz a "" b " " c 2>&1 | > + sed -ne "/run_command:/s/.*trace: run_command: //p" >actual && > + echo "git-frotz a '\'''\'' b '\'' '\'' c" >expect && > + test_cmp expect actual > +' > + > test_done
diff --git a/quote.c b/quote.c index 7f2aa6faa4..26f1848dde 100644 --- a/quote.c +++ b/quote.c @@ -48,6 +48,15 @@ void sq_quote_buf_pretty(struct strbuf *dst, const char *src) static const char ok_punct[] = "+,-./:=@_^"; const char *p; + if (!src) + BUG("Cannot append a NULL token to the buffer"); + + /* Avoid losing a zero-length string by adding '' */ + if (!*src) { + strbuf_addstr(dst, "''"); + return; + } + for (p = src; *p; p++) { if (!isalpha(*p) && !isdigit(*p) && !strchr(ok_punct, *p)) { sq_quote_buf(dst, src); diff --git a/t/t0014-alias.sh b/t/t0014-alias.sh index a070e645d7..2694c81afd 100755 --- a/t/t0014-alias.sh +++ b/t/t0014-alias.sh @@ -37,4 +37,11 @@ test_expect_success 'looping aliases - internal execution' ' # test_i18ngrep "^fatal: alias loop detected: expansion of" output #' +test_expect_success 'run-command formats empty args properly' ' + GIT_TRACE=1 git frotz a "" b " " c 2>&1 | + sed -ne "/run_command:/s/.*trace: run_command: //p" >actual && + echo "git-frotz a '\'''\'' b '\'' '\'' c" >expect && + test_cmp expect actual +' + test_done