mirror/git
1
0
Fork 0
mirror of https://github.com/git/git.git synced 2024-11-19 05:24:21 +01:00
Code

run_column_filter: use argv_array

Browse Source 
We currently set up the argv array by hand in a fixed-size
stack-local array. Using an argv array is more readable, as
it handles buffer allocation us (not to mention makes it
obvious we do not overflow the array).

However, there's a more subtle benefit, too. We leave the
function having run start_command (with the child_process
in a static global), and then later run finish_command from
another function. That means when we run finish_command,
neither column_process.argv nor the memory it points to is
valid any longer.

Most of the time finish_command does not bother looking at
argv, but it may if it encounters an error (e.g., waitpid
failure or signal death). This is unusual, which is why
nobody has noticed. But by using run-command's built-in
argv_array, the memory ownership is handled for us
automatically.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Jeff King 2014-05-15 04:33:40 -04:00 committed by Junio C Hamano
parent c460c0ecdc
commit 5eb7f7ead8
1 changed files with 13 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
column.c
View File

@ -370,46 +370,29 @@ static struct child_process column_process;
int run_column_filter(int colopts, const struct column_options *opts)
{
const char *av[10];
int ret, ac = 0;
struct strbuf sb_colopt = STRBUF_INIT;
struct strbuf sb_width = STRBUF_INIT;
struct strbuf sb_padding = STRBUF_INIT;
struct argv_array *argv;
if (fd_out != -1)
return -1;
av[ac++] = "column";
strbuf_addf(&sb_colopt, "--raw-mode=%d", colopts);
av[ac++] = sb_colopt.buf;
if (opts && opts->width) {
strbuf_addf(&sb_width, "--width=%d", opts->width);
av[ac++] = sb_width.buf;
}
if (opts && opts->indent) {
av[ac++] = "--indent";
av[ac++] = opts->indent;
}
if (opts && opts->padding) {
strbuf_addf(&sb_padding, "--padding=%d", opts->padding);
av[ac++] = sb_padding.buf;
}
av[ac] = NULL;
memset(&column_process, 0, sizeof(column_process));
argv = &column_process.args;
argv_array_push(argv, "column");
argv_array_pushf(argv, "--raw-mode=%d", colopts);
if (opts && opts->width)
argv_array_pushf(argv, "--width=%d", opts->width);
if (opts && opts->indent)
argv_array_pushf(argv, "--indent=%s", opts->indent);
if (opts && opts->padding)
argv_array_pushf(argv, "--padding=%d", opts->padding);
fflush(stdout);
memset(&column_process, 0, sizeof(column_process));
column_process.in = -1;
column_process.out = dup(1);
column_process.git_cmd = 1;
column_process.argv = av;
ret = start_command(&column_process);
strbuf_release(&sb_colopt);
strbuf_release(&sb_width);
strbuf_release(&sb_padding);
if (ret)
if (start_command(&column_process))
return -2;
fd_out = dup(1);