@matheusbsbs / matheus-bagliano / commits / 02e8ca0e50

parse_dirstat_params(): use string_list to split comma-separated string

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>

Michael Haggerty committed Oct 28, 2012 at 17:50 UTC 02e8ca0e50abc15596db1fa93bda29bbd91007a7
1 file changed +21 -19
diff.c
+21 -19
@@ -15,6 +15,7 @@
15 #include "sigchain.h"
16 #include "submodule.h"
17 #include "ll-merge.h"
18 +#include "string-list.h"
19
20 #ifdef NO_FAST_WORKING_DIRECTORY
21 #define FAST_WORKING_DIRECTORY 0
@@ -68,26 +69,30 @@ static int parse_diff_color_slot(const char *var, int ofs)
69 return -1;
70 }
71
71 -static int parse_dirstat_params(struct diff_options *options, const char *params,
72 +static int parse_dirstat_params(struct diff_options *options, const char *params_string,
73 struct strbuf *errmsg)
74 {
74 - const char *p = params;
75 - int p_len, ret = 0;
75 + char *params_copy = xstrdup(params_string);
76 + struct string_list params = STRING_LIST_INIT_NODUP;
77 + int ret = 0;
78 + int i;
79
77 - while (*p) {
78 - p_len = strchrnul(p, ',') - p;
79 - if (!memcmp(p, "changes", p_len)) {
80 + if (*params_copy)
81 + string_list_split_in_place(&params, params_copy, ',', -1);
82 + for (i = 0; i < params.nr; i++) {
83 + const char *p = params.items[i].string;
84 + if (!strcmp(p, "changes")) {
85 DIFF_OPT_CLR(options, DIRSTAT_BY_LINE);
86 DIFF_OPT_CLR(options, DIRSTAT_BY_FILE);
82 - } else if (!memcmp(p, "lines", p_len)) {
87 + } else if (!strcmp(p, "lines")) {
88 DIFF_OPT_SET(options, DIRSTAT_BY_LINE);
89 DIFF_OPT_CLR(options, DIRSTAT_BY_FILE);
85 - } else if (!memcmp(p, "files", p_len)) {
90 + } else if (!strcmp(p, "files")) {
91 DIFF_OPT_CLR(options, DIRSTAT_BY_LINE);
92 DIFF_OPT_SET(options, DIRSTAT_BY_FILE);
88 - } else if (!memcmp(p, "noncumulative", p_len)) {
93 + } else if (!strcmp(p, "noncumulative")) {
94 DIFF_OPT_CLR(options, DIRSTAT_CUMULATIVE);
90 - } else if (!memcmp(p, "cumulative", p_len)) {
95 + } else if (!strcmp(p, "cumulative")) {
96 DIFF_OPT_SET(options, DIRSTAT_CUMULATIVE);
97 } else if (isdigit(*p)) {
98 char *end;
@@ -99,24 +104,21 @@ static int parse_dirstat_params(struct diff_options *options, const char *params
104 while (isdigit(*++end))
105 ; /* nothing */
106 }
102 - if (end - p == p_len)
107 + if (!*end)
108 options->dirstat_permille = permille;
109 else {
105 - strbuf_addf(errmsg, _(" Failed to parse dirstat cut-off percentage '%.*s'\n"),
106 - p_len, p);
110 + strbuf_addf(errmsg, _(" Failed to parse dirstat cut-off percentage '%s'\n"),
111 + p);
112 ret++;
113 }
114 } else {
110 - strbuf_addf(errmsg, _(" Unknown dirstat parameter '%.*s'\n"),
111 - p_len, p);
115 + strbuf_addf(errmsg, _(" Unknown dirstat parameter '%s'\n"), p);
116 ret++;
117 }
118
115 - p += p_len;
116 -
117 - if (*p)
118 - p++; /* more parameters, swallow separator */
119 }
120 + string_list_clear(&params, 0);
121 + free(params_copy);
122 return ret;
123 }
124