# [PATCH 0/3] small quote.[ch] cleanup 6 messages from 2026-05-19 to 2026-05-21. Participants: Jeff King, Junio C Hamano, Patrick Steinhardt. Thread: https://gitlist.dev/t/65655 ## Jeff King, 2026-05-19 01:18 Subject: [PATCH 0/3] small quote.[ch] cleanup Message-ID: <20260519011837.GA1615637@coredump.intra.peff.net> ``` I noticed some unused code while looking at an unrelated topic. So here's a small cleanup. [1/3]: quote.h: bump strvec forward declaration to the top [2/3]: quote: drop sq_dequote_to_argv() [3/3]: quote: simplify internals of dequoting quote.c | 21 ++------------------- quote.h | 14 ++++---------- 2 files changed, 6 insertions(+), 29 deletions(-) -Peff ``` ## Jeff King, 2026-05-19 01:19 Subject: [PATCH 1/3] quote.h: bump strvec forward declaration to the top Message-ID: <20260519011901.GA1615870@coredump.intra.peff.net> In-Reply-To: <20260519011837.GA1615637@coredump.intra.peff.net> ``` We usually put forward declarations at the top of header files, rather than next to the functions that need them. In theory placing it next to the function has some explanatory value, but it's also just as likely to become stale if other uses are added. Signed-off-by: Jeff King --- quote.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/quote.h b/quote.h index 0300c29104..400397b11a 100644 --- a/quote.h +++ b/quote.h @@ -2,6 +2,7 @@ #define QUOTE_H struct strbuf; +struct strvec; extern int quote_path_fully; @@ -77,7 +78,6 @@ int sq_dequote_to_argv(char *arg, const char ***argv, int *nr, int *alloc); * still modify arg in place, but unlike sq_dequote_to_argv, the strvec * will duplicate and take ownership of the strings. */ -struct strvec; int sq_dequote_to_strvec(char *arg, struct strvec *); int unquote_c_style(struct strbuf *, const char *quoted, const char **endp); -- 2.54.0.524.g198262df96 ``` ## Jeff King, 2026-05-19 01:19 Subject: [PATCH 2/3] quote: drop sq_dequote_to_argv() Message-ID: <20260519011934.GB1615870@coredump.intra.peff.net> In-Reply-To: <20260519011837.GA1615637@coredump.intra.peff.net> ``` The last caller went away in f9dbb64fad (config: parse more robust format in GIT_CONFIG_PARAMETERS, 2021-01-12), when we switched to using sq_dequote_step(). The "to_argv()" form is not a great interface. If you care about raw speed, then sq_dequote_step() lets you work incrementally without extra allocations. If you care about simplicity, then sq_dequote_to_strvec() puts the result in an encapsulated data structure. With sq_dequote_to_argv(), you have a data dependency on the original string but still have to remember to manually free the argv array itself (but not its elements). So it's sort of a worst-of-both-worlds middle ground. Let's get rid of it. Signed-off-by: Jeff King --- quote.c | 5 ----- quote.h | 12 +++--------- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/quote.c b/quote.c index b9f6bdc775..cff78af3a4 100644 --- a/quote.c +++ b/quote.c @@ -202,11 +202,6 @@ static int sq_dequote_to_argv_internal(char *arg, return 0; } -int sq_dequote_to_argv(char *arg, const char ***argv, int *nr, int *alloc) -{ - return sq_dequote_to_argv_internal(arg, argv, nr, alloc, NULL); -} - int sq_dequote_to_strvec(char *arg, struct strvec *array) { return sq_dequote_to_argv_internal(arg, NULL, NULL, NULL, array); diff --git a/quote.h b/quote.h index 400397b11a..989f2388c0 100644 --- a/quote.h +++ b/quote.h @@ -68,15 +68,9 @@ char *sq_dequote_step(char *src, char **next); /* * Same as the above, but can be used to unwrap many arguments in the - * same string separated by space. Like sq_quote, it works in place, - * modifying arg and appending pointers into it to argv. - */ -int sq_dequote_to_argv(char *arg, const char ***argv, int *nr, int *alloc); - -/* - * Same as above, but store the unquoted strings in a strvec. We will - * still modify arg in place, but unlike sq_dequote_to_argv, the strvec - * will duplicate and take ownership of the strings. + * same string separated by space. The strvec will duplicate and take + * ownership of the strings, but note that "arg" is still modified in-place + * during parsing. */ int sq_dequote_to_strvec(char *arg, struct strvec *); -- 2.54.0.524.g198262df96 ``` ## Jeff King, 2026-05-19 01:20 Subject: [PATCH 3/3] quote: simplify internals of dequoting Message-ID: <20260519012059.GC1615870@coredump.intra.peff.net> In-Reply-To: <20260519011837.GA1615637@coredump.intra.peff.net> ``` Our sq_dequote_to_argv_internal() helper was wrapped by the to_argv() and to_strvec() forms. Now that we have only the latter, we can stop wrapping it and drop the argv-only bits. Note that in theory sq_dequote_to_strvec() could take a const input string, which would be friendlier to its callers. We couldn't do that with the to_argv() form because it reused the input string to hold the output elements. But since we're built on sq_dequote_step(), which munges the input, we'd have to rework the parser. Since no callers care about it currently, we'll leave that for another day. Signed-off-by: Jeff King --- quote.c | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/quote.c b/quote.c index cff78af3a4..235fac8e47 100644 --- a/quote.c +++ b/quote.c @@ -171,9 +171,7 @@ char *sq_dequote(char *arg) return sq_dequote_step(arg, NULL); } -static int sq_dequote_to_argv_internal(char *arg, - const char ***argv, int *nr, int *alloc, - struct strvec *array) +int sq_dequote_to_strvec(char *arg, struct strvec *array) { char *next = arg; @@ -191,22 +189,12 @@ static int sq_dequote_to_argv_internal(char *arg, c = *++next; } while (isspace(c)); } - if (argv) { - ALLOC_GROW(*argv, *nr + 1, *alloc); - (*argv)[(*nr)++] = dequoted; - } - if (array) - strvec_push(array, dequoted); + strvec_push(array, dequoted); } while (next); return 0; } -int sq_dequote_to_strvec(char *arg, struct strvec *array) -{ - return sq_dequote_to_argv_internal(arg, NULL, NULL, NULL, array); -} - /* 1 means: quote as octal * 0 means: quote as octal if (quote_path_fully) * -1 means: never quote -- 2.54.0.524.g198262df96 ``` ## Junio C Hamano, 2026-05-19 03:19 Subject: Re: [PATCH 0/3] small quote.[ch] cleanup Message-ID: In-Reply-To: <20260519011837.GA1615637@coredump.intra.peff.net> ``` Jeff King writes: > I noticed some unused code while looking at an unrelated topic. So > here's a small cleanup. > > [1/3]: quote.h: bump strvec forward declaration to the top > [2/3]: quote: drop sq_dequote_to_argv() > [3/3]: quote: simplify internals of dequoting > > quote.c | 21 ++------------------- > quote.h | 14 ++++---------- > 2 files changed, 6 insertions(+), 29 deletions(-) > > -Peff These were very straight-forward and pleasant to read. Queued. Thanks. ``` ## Patrick Steinhardt, 2026-05-21 11:39 Subject: Re: [PATCH 0/3] small quote.[ch] cleanup Message-ID: In-Reply-To: ``` On Tue, May 19, 2026 at 12:19:20PM +0900, Junio C Hamano wrote: > Jeff King writes: > > > I noticed some unused code while looking at an unrelated topic. So > > here's a small cleanup. > > > > [1/3]: quote.h: bump strvec forward declaration to the top > > [2/3]: quote: drop sq_dequote_to_argv() > > [3/3]: quote: simplify internals of dequoting > > > > quote.c | 21 ++------------------- > > quote.h | 14 ++++---------- > > 2 files changed, 6 insertions(+), 29 deletions(-) > > > > -Peff > > These were very straight-forward and pleasant to read. Queued. > Thanks. Indeed, all of these steps make sense to me. Thanks! Patrick ```