Skip to content

Commit

Permalink
parse_dirstat_params(): use string_list to split comma-separated string
Browse files Browse the repository at this point in the history
Use string_list_split_in_place() to split the comma-separated
parameters string.  This simplifies the code and also fixes a bug: the
old code made calls like

    memcmp(p, "lines", p_len)

which needn't work if p_len is different than the length of the
constant string (and could illegally access memory if p_len is larger
than the length of the constant string).

When p_len was less than the length of the constant string, the old
code would have allowed some abbreviations to be accepted (e.g., "cha"
for "changes") but this seems to have been a bug rather than a
feature, because (1) it is not documented; (2) no attempt was made to
handle ambiguous abbreviations, like "c" for "changes" vs
"cumulative".

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Jeff King <peff@peff.net>
  • Loading branch information
Michael Haggerty authored and Jeff King committed Oct 29, 2012
1 parent 7e20105 commit 02e8ca0
Showing 1 changed file with 21 additions and 19 deletions.
40 changes: 21 additions & 19 deletions diff.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "sigchain.h"
#include "submodule.h"
#include "ll-merge.h"
#include "string-list.h"

#ifdef NO_FAST_WORKING_DIRECTORY
#define FAST_WORKING_DIRECTORY 0
Expand Down Expand Up @@ -68,26 +69,30 @@ static int parse_diff_color_slot(const char *var, int ofs)
return -1;
}

static int parse_dirstat_params(struct diff_options *options, const char *params,
static int parse_dirstat_params(struct diff_options *options, const char *params_string,
struct strbuf *errmsg)
{
const char *p = params;
int p_len, ret = 0;
char *params_copy = xstrdup(params_string);
struct string_list params = STRING_LIST_INIT_NODUP;
int ret = 0;
int i;

while (*p) {
p_len = strchrnul(p, ',') - p;
if (!memcmp(p, "changes", p_len)) {
if (*params_copy)
string_list_split_in_place(&params, params_copy, ',', -1);
for (i = 0; i < params.nr; i++) {
const char *p = params.items[i].string;
if (!strcmp(p, "changes")) {
DIFF_OPT_CLR(options, DIRSTAT_BY_LINE);
DIFF_OPT_CLR(options, DIRSTAT_BY_FILE);
} else if (!memcmp(p, "lines", p_len)) {
} else if (!strcmp(p, "lines")) {
DIFF_OPT_SET(options, DIRSTAT_BY_LINE);
DIFF_OPT_CLR(options, DIRSTAT_BY_FILE);
} else if (!memcmp(p, "files", p_len)) {
} else if (!strcmp(p, "files")) {
DIFF_OPT_CLR(options, DIRSTAT_BY_LINE);
DIFF_OPT_SET(options, DIRSTAT_BY_FILE);
} else if (!memcmp(p, "noncumulative", p_len)) {
} else if (!strcmp(p, "noncumulative")) {
DIFF_OPT_CLR(options, DIRSTAT_CUMULATIVE);
} else if (!memcmp(p, "cumulative", p_len)) {
} else if (!strcmp(p, "cumulative")) {
DIFF_OPT_SET(options, DIRSTAT_CUMULATIVE);
} else if (isdigit(*p)) {
char *end;
Expand All @@ -99,24 +104,21 @@ static int parse_dirstat_params(struct diff_options *options, const char *params
while (isdigit(*++end))
; /* nothing */
}
if (end - p == p_len)
if (!*end)
options->dirstat_permille = permille;
else {
strbuf_addf(errmsg, _(" Failed to parse dirstat cut-off percentage '%.*s'\n"),
p_len, p);
strbuf_addf(errmsg, _(" Failed to parse dirstat cut-off percentage '%s'\n"),
p);
ret++;
}
} else {
strbuf_addf(errmsg, _(" Unknown dirstat parameter '%.*s'\n"),
p_len, p);
strbuf_addf(errmsg, _(" Unknown dirstat parameter '%s'\n"), p);
ret++;
}

p += p_len;

if (*p)
p++; /* more parameters, swallow separator */
}
string_list_clear(&params, 0);
free(params_copy);
return ret;
}

Expand Down

0 comments on commit 02e8ca0

Please sign in to comment.