Skip to content

Commit 0fd2a2e

Browse files
gitstergitster
committed
Merge branch 'rs/parse-options-precision'
Define .precision to more canned parse-options type to avoid bugs coming from using a variable with a wrong type to capture the parsed values. * rs/parse-options-precision: parse-options: add precision handling for OPTION_COUNTUP parse-options: add precision handling for OPTION_BITOP parse-options: add precision handling for OPTION_NEGBIT parse-options: add precision handling for OPTION_BIT parse-options: add precision handling for OPTION_SET_INT parse-options: add precision handling for PARSE_OPT_CMDMODE parse-options: require PARSE_OPT_NOARG for OPTION_BITOP
2 parents edb4fd9 + c1e616c commit 0fd2a2e

File tree

7 files changed

+146
-42
lines changed

7 files changed

+146
-42
lines changed

‎builtin/am.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2406,6 +2406,7 @@ int cmd_am(int argc,
24062406
.type = OPTION_CALLBACK,
24072407
.long_name = "show-current-patch",
24082408
.value = &resume_mode,
2409+
.precision = sizeof(resume_mode),
24092410
.argh = "(diff|raw)",
24102411
.help = N_("show the patch being applied"),
24112412
.flags = PARSE_OPT_CMDMODE | PARSE_OPT_OPTARG | PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,

‎builtin/rebase.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1128,6 +1128,7 @@ int cmd_rebase(int argc,
11281128
.short_name = 'n',
11291129
.long_name = "no-stat",
11301130
.value = &options.flags,
1131+
.precision = sizeof(options.flags),
11311132
.help = N_("do not show diffstat of what changed upstream"),
11321133
.flags = PARSE_OPT_NOARG,
11331134
.defval = REBASE_DIFFSTAT,

‎builtin/update-index.c‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -981,6 +981,7 @@ int cmd_update_index(int argc,
981981
.type = OPTION_SET_INT,
982982
.long_name = "assume-unchanged",
983983
.value = &mark_valid_only,
984+
.precision = sizeof(mark_valid_only),
984985
.help = N_("mark files as \"not changing\""),
985986
.flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG,
986987
.defval = MARK_FLAG,
@@ -989,6 +990,7 @@ int cmd_update_index(int argc,
989990
.type = OPTION_SET_INT,
990991
.long_name = "no-assume-unchanged",
991992
.value = &mark_valid_only,
993+
.precision = sizeof(mark_valid_only),
992994
.help = N_("clear assumed-unchanged bit"),
993995
.flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG,
994996
.defval = UNMARK_FLAG,
@@ -997,6 +999,7 @@ int cmd_update_index(int argc,
997999
.type = OPTION_SET_INT,
9981000
.long_name = "skip-worktree",
9991001
.value = &mark_skip_worktree_only,
1002+
.precision = sizeof(mark_skip_worktree_only),
10001003
.help = N_("mark files as \"index-only\""),
10011004
.flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG,
10021005
.defval = MARK_FLAG,
@@ -1005,6 +1008,7 @@ int cmd_update_index(int argc,
10051008
.type = OPTION_SET_INT,
10061009
.long_name = "no-skip-worktree",
10071010
.value = &mark_skip_worktree_only,
1011+
.precision = sizeof(mark_skip_worktree_only),
10081012
.help = N_("clear skip-worktree bit"),
10091013
.flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG,
10101014
.defval = UNMARK_FLAG,
@@ -1079,6 +1083,7 @@ int cmd_update_index(int argc,
10791083
.type = OPTION_SET_INT,
10801084
.long_name = "fsmonitor-valid",
10811085
.value = &mark_fsmonitor_only,
1086+
.precision = sizeof(mark_fsmonitor_only),
10821087
.help = N_("mark files as fsmonitor valid"),
10831088
.flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG,
10841089
.defval = MARK_FLAG,
@@ -1087,6 +1092,7 @@ int cmd_update_index(int argc,
10871092
.type = OPTION_SET_INT,
10881093
.long_name = "no-fsmonitor-valid",
10891094
.value = &mark_fsmonitor_only,
1095+
.precision = sizeof(mark_fsmonitor_only),
10901096
.help = N_("clear fsmonitor valid bit"),
10911097
.flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG,
10921098
.defval = UNMARK_FLAG,

‎builtin/write-tree.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ int cmd_write_tree(int argc,
3535
.type = OPTION_BIT,
3636
.long_name = "ignore-cache-tree",
3737
.value = &flags,
38+
.precision = sizeof(flags),
3839
.help = N_("only useful for debugging"),
3940
.flags = PARSE_OPT_HIDDEN | PARSE_OPT_NOARG,
4041
.defval = WRITE_TREE_IGNORE_CACHE_TREE,

‎parse-options.c‎

Lines changed: 116 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,64 @@ static char *fix_filename(const char *prefix, const char *file)
6868
return prefix_filename_except_for_dash(prefix, file);
6969
}
7070

71+
static int do_get_int_value(const void *value, size_t precision, intmax_t *ret)
72+
{
73+
switch (precision) {
74+
case sizeof(int8_t):
75+
*ret = *(int8_t *)value;
76+
return 0;
77+
case sizeof(int16_t):
78+
*ret = *(int16_t *)value;
79+
return 0;
80+
case sizeof(int32_t):
81+
*ret = *(int32_t *)value;
82+
return 0;
83+
case sizeof(int64_t):
84+
*ret = *(int64_t *)value;
85+
return 0;
86+
default:
87+
return -1;
88+
}
89+
}
90+
91+
static intmax_t get_int_value(const struct option *opt, enum opt_parsed flags)
92+
{
93+
intmax_t ret;
94+
if (do_get_int_value(opt->value, opt->precision, &ret))
95+
BUG("invalid precision for option %s", optname(opt, flags));
96+
return ret;
97+
}
98+
99+
static enum parse_opt_result set_int_value(const struct option *opt,
100+
enum opt_parsed flags,
101+
intmax_t value)
102+
{
103+
switch (opt->precision) {
104+
case sizeof(int8_t):
105+
*(int8_t *)opt->value = value;
106+
return 0;
107+
case sizeof(int16_t):
108+
*(int16_t *)opt->value = value;
109+
return 0;
110+
case sizeof(int32_t):
111+
*(int32_t *)opt->value = value;
112+
return 0;
113+
case sizeof(int64_t):
114+
*(int64_t *)opt->value = value;
115+
return 0;
116+
default:
117+
BUG("invalid precision for option %s", optname(opt, flags));
118+
}
119+
}
120+
121+
static int signed_int_fits(intmax_t value, size_t precision)
122+
{
123+
size_t bits = precision * CHAR_BIT;
124+
intmax_t upper_bound = INTMAX_MAX >> (bitsizeof(intmax_t) - bits);
125+
intmax_t lower_bound = -upper_bound - 1;
126+
return lower_bound <= value && value <= upper_bound;
127+
}
128+
71129
static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
72130
const struct option *opt,
73131
enum opt_parsed flags,
@@ -89,35 +147,55 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
89147
return opt->ll_callback(p, opt, NULL, unset);
90148

91149
case OPTION_BIT:
150+
{
151+
intmax_t value = get_int_value(opt, flags);
92152
if (unset)
93-
*(int *)opt->value &= ~opt->defval;
153+
value &= ~opt->defval;
94154
else
95-
*(int *)opt->value |= opt->defval;
96-
return 0;
155+
value |= opt->defval;
156+
return set_int_value(opt, flags, value);
157+
}
97158

98159
case OPTION_NEGBIT:
160+
{
161+
intmax_t value = get_int_value(opt, flags);
99162
if (unset)
100-
*(int *)opt->value |= opt->defval;
163+
value |= opt->defval;
101164
else
102-
*(int *)opt->value &= ~opt->defval;
103-
return 0;
165+
value &= ~opt->defval;
166+
return set_int_value(opt, flags, value);
167+
}
104168

105169
case OPTION_BITOP:
170+
{
171+
intmax_t value = get_int_value(opt, flags);
106172
if (unset)
107173
BUG("BITOP can't have unset form");
108-
*(int *)opt->value &= ~opt->extra;
109-
*(int *)opt->value |= opt->defval;
110-
return 0;
174+
value &= ~opt->extra;
175+
value |= opt->defval;
176+
return set_int_value(opt, flags, value);
177+
}
111178

112179
case OPTION_COUNTUP:
113-
if (*(int *)opt->value < 0)
114-
*(int *)opt->value = 0;
115-
*(int *)opt->value = unset ? 0 : *(int *)opt->value + 1;
116-
return 0;
180+
{
181+
size_t bits = CHAR_BIT * opt->precision;
182+
intmax_t upper_bound = INTMAX_MAX >> (bitsizeof(intmax_t) - bits);
183+
intmax_t value = get_int_value(opt, flags);
184+
185+
if (value < 0)
186+
value = 0;
187+
if (unset)
188+
value = 0;
189+
else if (value < upper_bound)
190+
value++;
191+
else
192+
return error(_("value for %s exceeds %"PRIdMAX),
193+
optname(opt, flags), upper_bound);
194+
return set_int_value(opt, flags, value);
195+
}
117196

118197
case OPTION_SET_INT:
119-
*(int *)opt->value = unset ? 0 : opt->defval;
120-
return 0;
198+
return set_int_value(opt, flags, unset ? 0 : opt->defval);
121199

122200
case OPTION_STRING:
123201
if (unset)
@@ -199,23 +277,7 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
199277
return error(_("value %s for %s not in range [%"PRIdMAX",%"PRIdMAX"]"),
200278
arg, optname(opt, flags), (intmax_t)lower_bound, (intmax_t)upper_bound);
201279

202-
switch (opt->precision) {
203-
case 1:
204-
*(int8_t *)opt->value = value;
205-
return 0;
206-
case 2:
207-
*(int16_t *)opt->value = value;
208-
return 0;
209-
case 4:
210-
*(int32_t *)opt->value = value;
211-
return 0;
212-
case 8:
213-
*(int64_t *)opt->value = value;
214-
return 0;
215-
default:
216-
BUG("invalid precision for option %s",
217-
optname(opt, flags));
218-
}
280+
return set_int_value(opt, flags, value);
219281
}
220282
case OPTION_UNSIGNED:
221283
{
@@ -266,7 +328,9 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
266328
}
267329

268330
struct parse_opt_cmdmode_list {
269-
int value, *value_ptr;
331+
intmax_t value;
332+
void *value_ptr;
333+
size_t precision;
270334
const struct option *opt;
271335
const char *arg;
272336
enum opt_parsed flags;
@@ -280,7 +344,7 @@ static void build_cmdmode_list(struct parse_opt_ctx_t *ctx,
280344

281345
for (; opts->type != OPTION_END; opts++) {
282346
struct parse_opt_cmdmode_list *elem = ctx->cmdmode_list;
283-
int *value_ptr = opts->value;
347+
void *value_ptr = opts->value;
284348

285349
if (!(opts->flags & PARSE_OPT_CMDMODE) || !value_ptr)
286350
continue;
@@ -292,10 +356,13 @@ static void build_cmdmode_list(struct parse_opt_ctx_t *ctx,
292356

293357
CALLOC_ARRAY(elem, 1);
294358
elem->value_ptr = value_ptr;
295-
elem->value = *value_ptr;
359+
elem->precision = opts->precision;
360+
if (do_get_int_value(value_ptr, opts->precision, &elem->value))
361+
optbug(opts, "has invalid precision");
296362
elem->next = ctx->cmdmode_list;
297363
ctx->cmdmode_list = elem;
298364
}
365+
BUG_if_bug("invalid 'struct option'");
299366
}
300367

301368
static char *optnamearg(const struct option *opt, const char *arg,
@@ -317,7 +384,13 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,
317384
char *opt_name, *other_opt_name;
318385

319386
for (; elem; elem = elem->next) {
320-
if (*elem->value_ptr == elem->value)
387+
intmax_t new_value;
388+
389+
if (do_get_int_value(elem->value_ptr, elem->precision,
390+
&new_value))
391+
BUG("impossible: invalid precision");
392+
393+
if (new_value == elem->value)
321394
continue;
322395

323396
if (elem->opt &&
@@ -327,7 +400,7 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,
327400
elem->opt = opt;
328401
elem->arg = arg;
329402
elem->flags = flags;
330-
elem->value = *elem->value_ptr;
403+
elem->value = new_value;
331404
}
332405

333406
if (result || !elem)
@@ -586,10 +659,14 @@ static void parse_options_check(const struct option *opts)
586659
opts->long_name && !(opts->flags & PARSE_OPT_NONEG))
587660
optbug(opts, "OPTION_SET_INT 0 should not be negatable");
588661
switch (opts->type) {
589-
case OPTION_COUNTUP:
662+
case OPTION_SET_INT:
590663
case OPTION_BIT:
591664
case OPTION_NEGBIT:
592-
case OPTION_SET_INT:
665+
case OPTION_BITOP:
666+
case OPTION_COUNTUP:
667+
if (!signed_int_fits(opts->defval, opts->precision))
668+
optbug(opts, "has invalid defval");
669+
/* fallthru */
593670
case OPTION_NUMBER:
594671
if ((opts->flags & PARSE_OPT_OPTARG) ||
595672
!(opts->flags & PARSE_OPT_NOARG))

‎parse-options.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ struct option {
172172
.short_name = (s), \
173173
.long_name = (l), \
174174
.value = (v), \
175+
.precision = sizeof(*v), \
175176
.help = (h), \
176177
.flags = PARSE_OPT_NOARG|(f), \
177178
.callback = NULL, \
@@ -182,6 +183,7 @@ struct option {
182183
.short_name = (s), \
183184
.long_name = (l), \
184185
.value = (v), \
186+
.precision = sizeof(*v), \
185187
.help = (h), \
186188
.flags = PARSE_OPT_NOARG|(f), \
187189
}
@@ -190,6 +192,7 @@ struct option {
190192
.short_name = (s), \
191193
.long_name = (l), \
192194
.value = (v), \
195+
.precision = sizeof(*v), \
193196
.help = (h), \
194197
.flags = PARSE_OPT_NOARG | (f), \
195198
.defval = (i), \
@@ -238,6 +241,7 @@ struct option {
238241
.short_name = (s), \
239242
.long_name = (l), \
240243
.value = (v), \
244+
.precision = sizeof(*v), \
241245
.help = (h), \
242246
.flags = PARSE_OPT_NOARG|PARSE_OPT_NONEG, \
243247
.defval = (set), \
@@ -248,6 +252,7 @@ struct option {
248252
.short_name = (s), \
249253
.long_name = (l), \
250254
.value = (v), \
255+
.precision = sizeof(*v), \
251256
.help = (h), \
252257
.flags = PARSE_OPT_NOARG, \
253258
.defval = (b), \
@@ -260,6 +265,7 @@ struct option {
260265
.short_name = (s), \
261266
.long_name = (l), \
262267
.value = (v), \
268+
.precision = sizeof(*v), \
263269
.help = (h), \
264270
.flags = PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, \
265271
.defval = 1, \
@@ -269,6 +275,7 @@ struct option {
269275
.short_name = (s), \
270276
.long_name = (l), \
271277
.value = (v), \
278+
.precision = sizeof(*v), \
272279
.help = (h), \
273280
.flags = PARSE_OPT_CMDMODE|PARSE_OPT_NOARG|PARSE_OPT_NONEG | (f), \
274281
.defval = (i), \

0 commit comments

Comments
 (0)