On Tue, Sep 15, 2015 at 11:28 AM, Jeff King <peff@xxxxxxxx> 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/
> 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@xxxxxxxx>
> ---
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html