Skip to content

Commit ae021d8

Browse files
peffgitster
peff
authored and
gitster
committed
use skip_prefix to avoid magic numbers
It's a common idiom to match a prefix and then skip past it with a magic number, like: if (starts_with(foo, "bar")) foo += 3; This is easy to get wrong, since you have to count the prefix string yourself, and there's no compiler check if the string changes. We can use skip_prefix to avoid the magic numbers here. Note that some of these conversions could be much shorter. For example: if (starts_with(arg, "--foo=")) { bar = arg + 6; continue; } could become: if (skip_prefix(arg, "--foo=", &bar)) continue; However, I have left it as: if (skip_prefix(arg, "--foo=", &v)) { bar = v; continue; } to visually match nearby cases which need to actually process the string. Like: if (skip_prefix(arg, "--foo=", &v)) { bar = atoi(v); continue; } Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 21a2d4a commit ae021d8

File tree

11 files changed

+149
-131
lines changed

11 files changed

+149
-131
lines changed

‎alias.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ static char *alias_val;
55

66
static int alias_lookup_cb(const char *k, const char *v, void *cb)
77
{
8-
if (starts_with(k, "alias.") && !strcmp(k + 6, alias_key)) {
8+
const char *name;
9+
if (skip_prefix(k, "alias.", &name) && !strcmp(name, alias_key)) {
910
if (!v)
1011
return config_error_nonbool(k);
1112
alias_val = xstrdup(v);

‎connect.c

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
129129
char *name;
130130
int len, name_len;
131131
char *buffer = packet_buffer;
132+
const char *arg;
132133

133134
len = packet_read(in, &src_buf, &src_len,
134135
packet_buffer, sizeof(packet_buffer),
@@ -140,12 +141,12 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
140141
if (!len)
141142
break;
142143

143-
if (len > 4 && starts_with(buffer, "ERR "))
144-
die("remote error: %s", buffer + 4);
144+
if (len > 4 && skip_prefix(buffer, "ERR ", &arg))
145+
die("remote error: %s", arg);
145146

146-
if (len == 48 && starts_with(buffer, "shallow ")) {
147-
if (get_sha1_hex(buffer + 8, old_sha1))
148-
die("protocol error: expected shallow sha-1, got '%s'", buffer + 8);
147+
if (len == 48 && skip_prefix(buffer, "shallow ", &arg)) {
148+
if (get_sha1_hex(arg, old_sha1))
149+
die("protocol error: expected shallow sha-1, got '%s'", arg);
149150
if (!shallow_points)
150151
die("repository on the other end cannot be shallow");
151152
sha1_array_append(shallow_points, old_sha1);

‎convert.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1121,9 +1121,9 @@ static int is_foreign_ident(const char *str)
11211121
{
11221122
int i;
11231123

1124-
if (!starts_with(str, "$Id: "))
1124+
if (!skip_prefix(str, "$Id: ", &str))
11251125
return 0;
1126-
for (i = 5; str[i]; i++) {
1126+
for (i = 0; str[i]; i++) {
11271127
if (isspace(str[i]) && str[i+1] != '$')
11281128
return 1;
11291129
}

‎daemon.c

Lines changed: 38 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -235,8 +235,10 @@ static int service_enabled;
235235

236236
static int git_daemon_config(const char *var, const char *value, void *cb)
237237
{
238-
if (starts_with(var, "daemon.") &&
239-
!strcmp(var + 7, service_looking_at->config_name)) {
238+
const char *service;
239+
240+
if (skip_prefix(var, "daemon.", &service) &&
241+
!strcmp(service, service_looking_at->config_name)) {
240242
service_enabled = git_config_bool(var, value);
241243
return 0;
242244
}
@@ -1133,16 +1135,17 @@ int main(int argc, char **argv)
11331135

11341136
for (i = 1; i < argc; i++) {
11351137
char *arg = argv[i];
1138+
const char *v;
11361139

1137-
if (starts_with(arg, "--listen=")) {
1138-
string_list_append(&listen_addr, xstrdup_tolower(arg + 9));
1140+
if (skip_prefix(arg, "--listen=", &v)) {
1141+
string_list_append(&listen_addr, xstrdup_tolower(v));
11391142
continue;
11401143
}
1141-
if (starts_with(arg, "--port=")) {
1144+
if (skip_prefix(arg, "--port=", &v)) {
11421145
char *end;
11431146
unsigned long n;
1144-
n = strtoul(arg+7, &end, 0);
1145-
if (arg[7] && !*end) {
1147+
n = strtoul(v, &end, 0);
1148+
if (*v && !*end) {
11461149
listen_port = n;
11471150
continue;
11481151
}
@@ -1168,20 +1171,20 @@ int main(int argc, char **argv)
11681171
export_all_trees = 1;
11691172
continue;
11701173
}
1171-
if (starts_with(arg, "--access-hook=")) {
1172-
access_hook = arg + 14;
1174+
if (skip_prefix(arg, "--access-hook=", &v)) {
1175+
access_hook = v;
11731176
continue;
11741177
}
1175-
if (starts_with(arg, "--timeout=")) {
1176-
timeout = atoi(arg+10);
1178+
if (skip_prefix(arg, "--timeout=", &v)) {
1179+
timeout = atoi(v);
11771180
continue;
11781181
}
1179-
if (starts_with(arg, "--init-timeout=")) {
1180-
init_timeout = atoi(arg+15);
1182+
if (skip_prefix(arg, "--init-timeout=", &v)) {
1183+
init_timeout = atoi(v);
11811184
continue;
11821185
}
1183-
if (starts_with(arg, "--max-connections=")) {
1184-
max_connections = atoi(arg+18);
1186+
if (skip_prefix(arg, "--max-connections=", &v)) {
1187+
max_connections = atoi(v);
11851188
if (max_connections < 0)
11861189
max_connections = 0; /* unlimited */
11871190
continue;
@@ -1190,16 +1193,16 @@ int main(int argc, char **argv)
11901193
strict_paths = 1;
11911194
continue;
11921195
}
1193-
if (starts_with(arg, "--base-path=")) {
1194-
base_path = arg+12;
1196+
if (skip_prefix(arg, "--base-path=", &v)) {
1197+
base_path = v;
11951198
continue;
11961199
}
11971200
if (!strcmp(arg, "--base-path-relaxed")) {
11981201
base_path_relaxed = 1;
11991202
continue;
12001203
}
1201-
if (starts_with(arg, "--interpolated-path=")) {
1202-
interpolated_path = arg+20;
1204+
if (skip_prefix(arg, "--interpolated-path=", &v)) {
1205+
interpolated_path = v;
12031206
continue;
12041207
}
12051208
if (!strcmp(arg, "--reuseaddr")) {
@@ -1210,41 +1213,41 @@ int main(int argc, char **argv)
12101213
user_path = "";
12111214
continue;
12121215
}
1213-
if (starts_with(arg, "--user-path=")) {
1214-
user_path = arg + 12;
1216+
if (skip_prefix(arg, "--user-path=", &v)) {
1217+
user_path = v;
12151218
continue;
12161219
}
1217-
if (starts_with(arg, "--pid-file=")) {
1218-
pid_file = arg + 11;
1220+
if (skip_prefix(arg, "--pid-file=", &v)) {
1221+
pid_file = v;
12191222
continue;
12201223
}
12211224
if (!strcmp(arg, "--detach")) {
12221225
detach = 1;
12231226
log_syslog = 1;
12241227
continue;
12251228
}
1226-
if (starts_with(arg, "--user=")) {
1227-
user_name = arg + 7;
1229+
if (skip_prefix(arg, "--user=", &v)) {
1230+
user_name = v;
12281231
continue;
12291232
}
1230-
if (starts_with(arg, "--group=")) {
1231-
group_name = arg + 8;
1233+
if (skip_prefix(arg, "--group=", &v)) {
1234+
group_name = v;
12321235
continue;
12331236
}
1234-
if (starts_with(arg, "--enable=")) {
1235-
enable_service(arg + 9, 1);
1237+
if (skip_prefix(arg, "--enable=", &v)) {
1238+
enable_service(v, 1);
12361239
continue;
12371240
}
1238-
if (starts_with(arg, "--disable=")) {
1239-
enable_service(arg + 10, 0);
1241+
if (skip_prefix(arg, "--disable=", &v)) {
1242+
enable_service(v, 0);
12401243
continue;
12411244
}
1242-
if (starts_with(arg, "--allow-override=")) {
1243-
make_service_overridable(arg + 17, 1);
1245+
if (skip_prefix(arg, "--allow-override=", &v)) {
1246+
make_service_overridable(v, 1);
12441247
continue;
12451248
}
1246-
if (starts_with(arg, "--forbid-override=")) {
1247-
make_service_overridable(arg + 18, 0);
1249+
if (skip_prefix(arg, "--forbid-override=", &v)) {
1250+
make_service_overridable(v, 0);
12481251
continue;
12491252
}
12501253
if (!strcmp(arg, "--informative-errors")) {

‎diff.c

Lines changed: 34 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,8 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
231231

232232
int git_diff_basic_config(const char *var, const char *value, void *cb)
233233
{
234+
const char *name;
235+
234236
if (!strcmp(var, "diff.renamelimit")) {
235237
diff_rename_limit_default = git_config_int(var, value);
236238
return 0;
@@ -239,8 +241,9 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
239241
if (userdiff_config(var, value) < 0)
240242
return -1;
241243

242-
if (starts_with(var, "diff.color.") || starts_with(var, "color.diff.")) {
243-
int slot = parse_diff_color_slot(var + 11);
244+
if (skip_prefix(var, "diff.color.", &name) ||
245+
skip_prefix(var, "color.diff.", &name)) {
246+
int slot = parse_diff_color_slot(name);
244247
if (slot < 0)
245248
return 0;
246249
if (!value)
@@ -2341,6 +2344,7 @@ static void builtin_diff(const char *name_a,
23412344
} else {
23422345
/* Crazy xdl interfaces.. */
23432346
const char *diffopts = getenv("GIT_DIFF_OPTS");
2347+
const char *v;
23442348
xpparam_t xpp;
23452349
xdemitconf_t xecfg;
23462350
struct emit_callback ecbdata;
@@ -2379,10 +2383,10 @@ static void builtin_diff(const char *name_a,
23792383
xdiff_set_find_func(&xecfg, pe->pattern, pe->cflags);
23802384
if (!diffopts)
23812385
;
2382-
else if (starts_with(diffopts, "--unified="))
2383-
xecfg.ctxlen = strtoul(diffopts + 10, NULL, 10);
2384-
else if (starts_with(diffopts, "-u"))
2385-
xecfg.ctxlen = strtoul(diffopts + 2, NULL, 10);
2386+
else if (skip_prefix(diffopts, "--unified=", &v))
2387+
xecfg.ctxlen = strtoul(v, NULL, 10);
2388+
else if (skip_prefix(diffopts, "-u", &v))
2389+
xecfg.ctxlen = strtoul(v, NULL, 10);
23862390
if (o->word_diff)
23872391
init_diff_words_data(&ecbdata, o, one, two);
23882392
xdi_diff_outf(&mf1, &mf2, fn_out_consume, &ecbdata,
@@ -3609,17 +3613,17 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
36093613
options->output_format |= DIFF_FORMAT_SHORTSTAT;
36103614
else if (!strcmp(arg, "-X") || !strcmp(arg, "--dirstat"))
36113615
return parse_dirstat_opt(options, "");
3612-
else if (starts_with(arg, "-X"))
3613-
return parse_dirstat_opt(options, arg + 2);
3614-
else if (starts_with(arg, "--dirstat="))
3615-
return parse_dirstat_opt(options, arg + 10);
3616+
else if (skip_prefix(arg, "-X", &arg))
3617+
return parse_dirstat_opt(options, arg);
3618+
else if (skip_prefix(arg, "--dirstat=", &arg))
3619+
return parse_dirstat_opt(options, arg);
36163620
else if (!strcmp(arg, "--cumulative"))
36173621
return parse_dirstat_opt(options, "cumulative");
36183622
else if (!strcmp(arg, "--dirstat-by-file"))
36193623
return parse_dirstat_opt(options, "files");
3620-
else if (starts_with(arg, "--dirstat-by-file=")) {
3624+
else if (skip_prefix(arg, "--dirstat-by-file=", &arg)) {
36213625
parse_dirstat_opt(options, "files");
3622-
return parse_dirstat_opt(options, arg + 18);
3626+
return parse_dirstat_opt(options, arg);
36233627
}
36243628
else if (!strcmp(arg, "--check"))
36253629
options->output_format |= DIFF_FORMAT_CHECKDIFF;
@@ -3669,9 +3673,9 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
36693673
DIFF_OPT_CLR(options, RENAME_EMPTY);
36703674
else if (!strcmp(arg, "--relative"))
36713675
DIFF_OPT_SET(options, RELATIVE_NAME);
3672-
else if (starts_with(arg, "--relative=")) {
3676+
else if (skip_prefix(arg, "--relative=", &arg)) {
36733677
DIFF_OPT_SET(options, RELATIVE_NAME);
3674-
options->prefix = arg + 11;
3678+
options->prefix = arg;
36753679
}
36763680

36773681
/* xdiff options */
@@ -3722,8 +3726,8 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
37223726
DIFF_OPT_CLR(options, FOLLOW_RENAMES);
37233727
else if (!strcmp(arg, "--color"))
37243728
options->use_color = 1;
3725-
else if (starts_with(arg, "--color=")) {
3726-
int value = git_config_colorbool(NULL, arg+8);
3729+
else if (skip_prefix(arg, "--color=", &arg)) {
3730+
int value = git_config_colorbool(NULL, arg);
37273731
if (value < 0)
37283732
return error("option `color' expects \"always\", \"auto\", or \"never\"");
37293733
options->use_color = value;
@@ -3734,29 +3738,28 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
37343738
options->use_color = 1;
37353739
options->word_diff = DIFF_WORDS_COLOR;
37363740
}
3737-
else if (starts_with(arg, "--color-words=")) {
3741+
else if (skip_prefix(arg, "--color-words=", &arg)) {
37383742
options->use_color = 1;
37393743
options->word_diff = DIFF_WORDS_COLOR;
3740-
options->word_regex = arg + 14;
3744+
options->word_regex = arg;
37413745
}
37423746
else if (!strcmp(arg, "--word-diff")) {
37433747
if (options->word_diff == DIFF_WORDS_NONE)
37443748
options->word_diff = DIFF_WORDS_PLAIN;
37453749
}
3746-
else if (starts_with(arg, "--word-diff=")) {
3747-
const char *type = arg + 12;
3748-
if (!strcmp(type, "plain"))
3750+
else if (skip_prefix(arg, "--word-diff=", &arg)) {
3751+
if (!strcmp(arg, "plain"))
37493752
options->word_diff = DIFF_WORDS_PLAIN;
3750-
else if (!strcmp(type, "color")) {
3753+
else if (!strcmp(arg, "color")) {
37513754
options->use_color = 1;
37523755
options->word_diff = DIFF_WORDS_COLOR;
37533756
}
3754-
else if (!strcmp(type, "porcelain"))
3757+
else if (!strcmp(arg, "porcelain"))
37553758
options->word_diff = DIFF_WORDS_PORCELAIN;
3756-
else if (!strcmp(type, "none"))
3759+
else if (!strcmp(arg, "none"))
37573760
options->word_diff = DIFF_WORDS_NONE;
37583761
else
3759-
die("bad --word-diff argument: %s", type);
3762+
die("bad --word-diff argument: %s", arg);
37603763
}
37613764
else if ((argcount = parse_long_opt("word-diff-regex", av, &optarg))) {
37623765
if (options->word_diff == DIFF_WORDS_NONE)
@@ -3779,13 +3782,13 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
37793782
else if (!strcmp(arg, "--ignore-submodules")) {
37803783
DIFF_OPT_SET(options, OVERRIDE_SUBMODULE_CONFIG);
37813784
handle_ignore_submodules_arg(options, "all");
3782-
} else if (starts_with(arg, "--ignore-submodules=")) {
3785+
} else if (skip_prefix(arg, "--ignore-submodules=", &arg)) {
37833786
DIFF_OPT_SET(options, OVERRIDE_SUBMODULE_CONFIG);
3784-
handle_ignore_submodules_arg(options, arg + 20);
3787+
handle_ignore_submodules_arg(options, arg);
37853788
} else if (!strcmp(arg, "--submodule"))
37863789
DIFF_OPT_SET(options, SUBMODULE_LOG);
3787-
else if (starts_with(arg, "--submodule="))
3788-
return parse_submodule_opt(options, arg + 12);
3790+
else if (skip_prefix(arg, "--submodule=", &arg))
3791+
return parse_submodule_opt(options, arg);
37893792

37903793
/* misc options */
37913794
else if (!strcmp(arg, "-z"))
@@ -3820,8 +3823,8 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
38203823
}
38213824
else if (!strcmp(arg, "--abbrev"))
38223825
options->abbrev = DEFAULT_ABBREV;
3823-
else if (starts_with(arg, "--abbrev=")) {
3824-
options->abbrev = strtoul(arg + 9, NULL, 10);
3826+
else if (skip_prefix(arg, "--abbrev=", &arg)) {
3827+
options->abbrev = strtoul(arg, NULL, 10);
38253828
if (options->abbrev < MINIMUM_ABBREV)
38263829
options->abbrev = MINIMUM_ABBREV;
38273830
else if (40 < options->abbrev)

0 commit comments

Comments
 (0)