Re: [PATCH 11/67] trace: use strbuf for quote_crnl output
From: Eric Sunshine <hidden>
Date: 2016-06-15 23:06:34
On Tue, Sep 15, 2015 at 11:28 AM, Jeff King [off-list ref] 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/
quoted hunk ↗ jump to hunk
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 <redacted> ---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?
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