Re: [PATCH 11/67] trace: use strbuf for quote_crnl output
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Sep 16, 2015, 00:55 UTC
- Message-ID
- <CAPig+cS3WFg1JtN7WU-SFSbT3a2L3o7wiZr8OMGJ5E3As00q3w@mail.gmail.com>
- In-Reply-To
- <20150915152843.GK29753@sigill.intra.peff.net>
On Tue, Sep 15, 2015 at 11:28 AM, Jeff King <peff@peff.net> wrote:
> When we output GIT_TRACE_SETUP paths, we quote any > meta-characters. But our buffer to hold the result is only > PATH_MAX bytes, and we could double the size of the input > path (if every character needs quoted). We could use a
s/quoted/to be &/ ...or... s/quoted/quoting/
Show 19 quoted lines
> 2*PATH_MAX buffer, if we assume the input will never be more
> than PATH_MAX. But it's easier still to just switch to a
> strbuf and not worry about whether the input can exceed
> PATH_MAX or not.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> diff --git a/trace.c b/trace.c
> index 7393926..0c06d71 100644
> --- a/trace.c
> +++ b/trace.c
> @@ -277,25 +277,25 @@ void trace_performance_fl(const char *file, int line, uint64_t nanos,
>
> static const char *quote_crnl(const char *path)
> {
> - static char new_path[PATH_MAX];
> + static struct strbuf new_path = STRBUF_INIT;
> const char *p2 = path;
> - char *p1 = new_path;It's a little sad that this leaves a variable named 'p2' when there is no corresponding 'p1'. Would this deserve a cleanup patch which renames 'p2' to 'p' or do we not care enough?
Show 27 quoted lines
> if (!path)
> return NULL;
>
> + strbuf_reset(&new_path);
> +
> while (*p2) {
> switch (*p2) {
> - case '\\': *p1++ = '\\'; *p1++ = '\\'; break;
> - case '\n': *p1++ = '\\'; *p1++ = 'n'; break;
> - case '\r': *p1++ = '\\'; *p1++ = 'r'; break;
> + case '\\': strbuf_addstr(&new_path, "\\\\"); break;
> + case '\n': strbuf_addstr(&new_path, "\\n"); break;
> + case '\r': strbuf_addstr(&new_path, "\\r"); break;
> default:
> - *p1++ = *p2;
> + strbuf_addch(&new_path, *p2);
> }
> p2++;
> }
> - *p1 = '\0';
> - return new_path;
> + return new_path.buf;
> }
>
> /* FIXME: move prefix to startup_info struct and get rid of this arg */
> --
> 2.6.0.rc2.408.ga2926b9