Skip to content

Commit f41179f

Browse files
pcloudsgitster
pclouds
authored and
gitster
committed
parse-options: avoid magic return codes
Give names to these magic negative numbers. Make parse_opt_ll_cb return an enum to make clear it can actually control parse_options() with different return values (parse_opt_cb can too, but nobody needs it). Signed-off-by: Nguyễn Thái Ngọc Duy <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent bf3ff33 commit f41179f

File tree

5 files changed

+68
-46
lines changed

5 files changed

+68
-46
lines changed

‎builtin/merge.c‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,9 @@ static int option_parse_message(const struct option *opt,
112112
return 0;
113113
}
114114

115-
static int option_read_message(struct parse_opt_ctx_t *ctx,
116-
const struct option *opt, int unset)
115+
static enum parse_opt_result option_read_message(struct parse_opt_ctx_t *ctx,
116+
const struct option *opt,
117+
int unset)
117118
{
118119
struct strbuf *buf = opt->value;
119120
const char *arg;

‎builtin/update-index.c‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -847,8 +847,8 @@ static int parse_new_style_cacheinfo(const char *arg,
847847
return 0;
848848
}
849849

850-
static int cacheinfo_callback(struct parse_opt_ctx_t *ctx,
851-
const struct option *opt, int unset)
850+
static enum parse_opt_result cacheinfo_callback(
851+
struct parse_opt_ctx_t *ctx, const struct option *opt, int unset)
852852
{
853853
struct object_id oid;
854854
unsigned int mode;
@@ -873,8 +873,8 @@ static int cacheinfo_callback(struct parse_opt_ctx_t *ctx,
873873
return 0;
874874
}
875875

876-
static int stdin_cacheinfo_callback(struct parse_opt_ctx_t *ctx,
877-
const struct option *opt, int unset)
876+
static enum parse_opt_result stdin_cacheinfo_callback(
877+
struct parse_opt_ctx_t *ctx, const struct option *opt, int unset)
878878
{
879879
int *nul_term_line = opt->value;
880880

@@ -887,8 +887,8 @@ static int stdin_cacheinfo_callback(struct parse_opt_ctx_t *ctx,
887887
return 0;
888888
}
889889

890-
static int stdin_callback(struct parse_opt_ctx_t *ctx,
891-
const struct option *opt, int unset)
890+
static enum parse_opt_result stdin_callback(
891+
struct parse_opt_ctx_t *ctx, const struct option *opt, int unset)
892892
{
893893
int *read_from_stdin = opt->value;
894894

@@ -900,8 +900,8 @@ static int stdin_callback(struct parse_opt_ctx_t *ctx,
900900
return 0;
901901
}
902902

903-
static int unresolve_callback(struct parse_opt_ctx_t *ctx,
904-
const struct option *opt, int unset)
903+
static enum parse_opt_result unresolve_callback(
904+
struct parse_opt_ctx_t *ctx, const struct option *opt, int unset)
905905
{
906906
int *has_errors = opt->value;
907907
const char *prefix = startup_info->prefix;
@@ -919,8 +919,8 @@ static int unresolve_callback(struct parse_opt_ctx_t *ctx,
919919
return 0;
920920
}
921921

922-
static int reupdate_callback(struct parse_opt_ctx_t *ctx,
923-
const struct option *opt, int unset)
922+
static enum parse_opt_result reupdate_callback(
923+
struct parse_opt_ctx_t *ctx, const struct option *opt, int unset)
924924
{
925925
int *has_errors = opt->value;
926926
const char *prefix = startup_info->prefix;

‎parse-options-cb.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -170,10 +170,10 @@ int parse_opt_noop_cb(const struct option *opt, const char *arg, int unset)
170170
* "-h" output even if it's not being handled directly by
171171
* parse_options().
172172
*/
173-
int parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx,
174-
const struct option *opt, int unset)
173+
enum parse_opt_result parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx,
174+
const struct option *opt, int unset)
175175
{
176-
return -2;
176+
return PARSE_OPT_UNKNOWN;
177177
}
178178

179179
/**

‎parse-options.c‎

Lines changed: 45 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,9 @@ int optbug(const struct option *opt, const char *reason)
2020
return error("BUG: switch '%c' %s", opt->short_name, reason);
2121
}
2222

23-
static int get_arg(struct parse_opt_ctx_t *p, const struct option *opt,
24-
int flags, const char **arg)
23+
static enum parse_opt_result get_arg(struct parse_opt_ctx_t *p,
24+
const struct option *opt,
25+
int flags, const char **arg)
2526
{
2627
if (p->opt) {
2728
*arg = p->opt;
@@ -44,9 +45,10 @@ static void fix_filename(const char *prefix, const char **file)
4445
*file = prefix_filename(prefix, *file);
4546
}
4647

47-
static int opt_command_mode_error(const struct option *opt,
48-
const struct option *all_opts,
49-
int flags)
48+
static enum parse_opt_result opt_command_mode_error(
49+
const struct option *opt,
50+
const struct option *all_opts,
51+
int flags)
5052
{
5153
const struct option *that;
5254
struct strbuf that_name = STRBUF_INIT;
@@ -69,16 +71,16 @@ static int opt_command_mode_error(const struct option *opt,
6971
error(_("%s is incompatible with %s"),
7072
optname(opt, flags), that_name.buf);
7173
strbuf_release(&that_name);
72-
return -1;
74+
return PARSE_OPT_ERROR;
7375
}
7476
return error(_("%s : incompatible with something else"),
7577
optname(opt, flags));
7678
}
7779

78-
static int get_value(struct parse_opt_ctx_t *p,
79-
const struct option *opt,
80-
const struct option *all_opts,
81-
int flags)
80+
static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,
81+
const struct option *opt,
82+
const struct option *all_opts,
83+
int flags)
8284
{
8385
const char *s, *arg;
8486
const int unset = flags & OPT_UNSET;
@@ -208,7 +210,8 @@ static int get_value(struct parse_opt_ctx_t *p,
208210
}
209211
}
210212

211-
static int parse_short_opt(struct parse_opt_ctx_t *p, const struct option *options)
213+
static enum parse_opt_result parse_short_opt(struct parse_opt_ctx_t *p,
214+
const struct option *options)
212215
{
213216
const struct option *all_opts = options;
214217
const struct option *numopt = NULL;
@@ -239,11 +242,12 @@ static int parse_short_opt(struct parse_opt_ctx_t *p, const struct option *optio
239242
free(arg);
240243
return rc;
241244
}
242-
return -2;
245+
return PARSE_OPT_UNKNOWN;
243246
}
244247

245-
static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
246-
const struct option *options)
248+
static enum parse_opt_result parse_long_opt(
249+
struct parse_opt_ctx_t *p, const char *arg,
250+
const struct option *options)
247251
{
248252
const struct option *all_opts = options;
249253
const char *arg_end = strchrnul(arg, '=');
@@ -269,7 +273,7 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
269273
if (*rest)
270274
continue;
271275
p->out[p->cpidx++] = arg - 2;
272-
return 0;
276+
return PARSE_OPT_DONE;
273277
}
274278
if (!rest) {
275279
/* abbreviated? */
@@ -334,11 +338,11 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
334338
ambiguous_option->long_name,
335339
(abbrev_flags & OPT_UNSET) ? "no-" : "",
336340
abbrev_option->long_name);
337-
return -3;
341+
return PARSE_OPT_HELP;
338342
}
339343
if (abbrev_option)
340344
return get_value(p, abbrev_option, all_opts, abbrev_flags);
341-
return -2;
345+
return PARSE_OPT_UNKNOWN;
342346
}
343347

344348
static int parse_nodash_opt(struct parse_opt_ctx_t *p, const char *arg,
@@ -590,22 +594,28 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
590594
if (arg[1] != '-') {
591595
ctx->opt = arg + 1;
592596
switch (parse_short_opt(ctx, options)) {
593-
case -1:
597+
case PARSE_OPT_ERROR:
594598
return PARSE_OPT_ERROR;
595-
case -2:
599+
case PARSE_OPT_UNKNOWN:
596600
if (ctx->opt)
597601
check_typos(arg + 1, options);
598602
if (internal_help && *ctx->opt == 'h')
599603
goto show_usage;
600604
goto unknown;
605+
case PARSE_OPT_NON_OPTION:
606+
case PARSE_OPT_HELP:
607+
case PARSE_OPT_COMPLETE:
608+
BUG("parse_short_opt() cannot return these");
609+
case PARSE_OPT_DONE:
610+
break;
601611
}
602612
if (ctx->opt)
603613
check_typos(arg + 1, options);
604614
while (ctx->opt) {
605615
switch (parse_short_opt(ctx, options)) {
606-
case -1:
616+
case PARSE_OPT_ERROR:
607617
return PARSE_OPT_ERROR;
608-
case -2:
618+
case PARSE_OPT_UNKNOWN:
609619
if (internal_help && *ctx->opt == 'h')
610620
goto show_usage;
611621

@@ -617,6 +627,12 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
617627
ctx->argv[0] = xstrdup(ctx->opt - 1);
618628
*(char *)ctx->argv[0] = '-';
619629
goto unknown;
630+
case PARSE_OPT_NON_OPTION:
631+
case PARSE_OPT_COMPLETE:
632+
case PARSE_OPT_HELP:
633+
BUG("parse_short_opt() cannot return these");
634+
case PARSE_OPT_DONE:
635+
break;
620636
}
621637
}
622638
continue;
@@ -635,12 +651,17 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
635651
if (internal_help && !strcmp(arg + 2, "help"))
636652
goto show_usage;
637653
switch (parse_long_opt(ctx, arg + 2, options)) {
638-
case -1:
654+
case PARSE_OPT_ERROR:
639655
return PARSE_OPT_ERROR;
640-
case -2:
656+
case PARSE_OPT_UNKNOWN:
641657
goto unknown;
642-
case -3:
658+
case PARSE_OPT_HELP:
643659
goto show_usage;
660+
case PARSE_OPT_NON_OPTION:
661+
case PARSE_OPT_COMPLETE:
662+
BUG("parse_long_opt() cannot return these");
663+
case PARSE_OPT_DONE:
664+
break;
644665
}
645666
continue;
646667
unknown:

‎parse-options.h‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,8 @@ struct option;
4949
typedef int parse_opt_cb(const struct option *, const char *arg, int unset);
5050

5151
struct parse_opt_ctx_t;
52-
typedef int parse_opt_ll_cb(struct parse_opt_ctx_t *ctx,
53-
const struct option *opt, int unset);
52+
typedef enum parse_opt_result parse_opt_ll_cb(struct parse_opt_ctx_t *ctx,
53+
const struct option *opt, int unset);
5454

5555
/*
5656
* `type`::
@@ -222,12 +222,12 @@ const char *optname(const struct option *opt, int flags);
222222

223223
/*----- incremental advanced APIs -----*/
224224

225-
enum {
226-
PARSE_OPT_COMPLETE = -2,
227-
PARSE_OPT_HELP = -1,
228-
PARSE_OPT_DONE,
225+
enum parse_opt_result {
226+
PARSE_OPT_COMPLETE = -3,
227+
PARSE_OPT_HELP = -2,
228+
PARSE_OPT_ERROR = -1, /* must be the same as error() */
229+
PARSE_OPT_DONE = 0, /* fixed so that "return 0" works */
229230
PARSE_OPT_NON_OPTION,
230-
PARSE_OPT_ERROR,
231231
PARSE_OPT_UNKNOWN
232232
};
233233

0 commit comments

Comments
 (0)