Skip to content

Commit a3da694

Browse files
pks-tgitster
pks-t
authored and
gitster
committed
remote-curl: avoid assigning string constant to non-const variable
When processing remote options, we split the option line into two by searching for a space. If there is one, we replace the space with '\0', otherwise we implicitly assume that the value is "true" and thus assign a string constant. As the return value of strchr(3P) weirdly enough is a `char *` even though it gets a `const char *` as input, the assigned-to variable also is a non-constant. This is fine though because the argument is in fact an allocated string, and thus we are allowed to modify it. But this will break once we enable `-Wwrite-strings`. Refactor the code stop splitting the fields with '\0' altogether. Instead, we can pass the length of the option name to `set_option()` and then use strncmp(3P) instead of strcmp(3P). Signed-off-by: Patrick Steinhardt <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 5bd0851 commit a3da694

File tree

1 file changed

+27
-26
lines changed

1 file changed

+27
-26
lines changed

‎remote-curl.c

Lines changed: 27 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -58,17 +58,17 @@ struct options {
5858
static struct options options;
5959
static struct string_list cas_options = STRING_LIST_INIT_DUP;
6060

61-
static int set_option(const char *name, const char *value)
61+
static int set_option(const char *name, size_t namelen, const char *value)
6262
{
63-
if (!strcmp(name, "verbosity")) {
63+
if (!strncmp(name, "verbosity", namelen)) {
6464
char *end;
6565
int v = strtol(value, &end, 10);
6666
if (value == end || *end)
6767
return -1;
6868
options.verbosity = v;
6969
return 0;
7070
}
71-
else if (!strcmp(name, "progress")) {
71+
else if (!strncmp(name, "progress", namelen)) {
7272
if (!strcmp(value, "true"))
7373
options.progress = 1;
7474
else if (!strcmp(value, "false"))
@@ -77,23 +77,23 @@ static int set_option(const char *name, const char *value)
7777
return -1;
7878
return 0;
7979
}
80-
else if (!strcmp(name, "depth")) {
80+
else if (!strncmp(name, "depth", namelen)) {
8181
char *end;
8282
unsigned long v = strtoul(value, &end, 10);
8383
if (value == end || *end)
8484
return -1;
8585
options.depth = v;
8686
return 0;
8787
}
88-
else if (!strcmp(name, "deepen-since")) {
88+
else if (!strncmp(name, "deepen-since", namelen)) {
8989
options.deepen_since = xstrdup(value);
9090
return 0;
9191
}
92-
else if (!strcmp(name, "deepen-not")) {
92+
else if (!strncmp(name, "deepen-not", namelen)) {
9393
string_list_append(&options.deepen_not, value);
9494
return 0;
9595
}
96-
else if (!strcmp(name, "deepen-relative")) {
96+
else if (!strncmp(name, "deepen-relative", namelen)) {
9797
if (!strcmp(value, "true"))
9898
options.deepen_relative = 1;
9999
else if (!strcmp(value, "false"))
@@ -102,7 +102,7 @@ static int set_option(const char *name, const char *value)
102102
return -1;
103103
return 0;
104104
}
105-
else if (!strcmp(name, "followtags")) {
105+
else if (!strncmp(name, "followtags", namelen)) {
106106
if (!strcmp(value, "true"))
107107
options.followtags = 1;
108108
else if (!strcmp(value, "false"))
@@ -111,7 +111,7 @@ static int set_option(const char *name, const char *value)
111111
return -1;
112112
return 0;
113113
}
114-
else if (!strcmp(name, "dry-run")) {
114+
else if (!strncmp(name, "dry-run", namelen)) {
115115
if (!strcmp(value, "true"))
116116
options.dry_run = 1;
117117
else if (!strcmp(value, "false"))
@@ -120,7 +120,7 @@ static int set_option(const char *name, const char *value)
120120
return -1;
121121
return 0;
122122
}
123-
else if (!strcmp(name, "check-connectivity")) {
123+
else if (!strncmp(name, "check-connectivity", namelen)) {
124124
if (!strcmp(value, "true"))
125125
options.check_self_contained_and_connected = 1;
126126
else if (!strcmp(value, "false"))
@@ -129,7 +129,7 @@ static int set_option(const char *name, const char *value)
129129
return -1;
130130
return 0;
131131
}
132-
else if (!strcmp(name, "cas")) {
132+
else if (!strncmp(name, "cas", namelen)) {
133133
struct strbuf val = STRBUF_INIT;
134134
strbuf_addstr(&val, "--force-with-lease=");
135135
if (*value != '"')
@@ -139,31 +139,31 @@ static int set_option(const char *name, const char *value)
139139
string_list_append(&cas_options, val.buf);
140140
strbuf_release(&val);
141141
return 0;
142-
} else if (!strcmp(name, TRANS_OPT_FORCE_IF_INCLUDES)) {
142+
} else if (!strncmp(name, TRANS_OPT_FORCE_IF_INCLUDES, namelen)) {
143143
if (!strcmp(value, "true"))
144144
options.force_if_includes = 1;
145145
else if (!strcmp(value, "false"))
146146
options.force_if_includes = 0;
147147
else
148148
return -1;
149149
return 0;
150-
} else if (!strcmp(name, "cloning")) {
150+
} else if (!strncmp(name, "cloning", namelen)) {
151151
if (!strcmp(value, "true"))
152152
options.cloning = 1;
153153
else if (!strcmp(value, "false"))
154154
options.cloning = 0;
155155
else
156156
return -1;
157157
return 0;
158-
} else if (!strcmp(name, "update-shallow")) {
158+
} else if (!strncmp(name, "update-shallow", namelen)) {
159159
if (!strcmp(value, "true"))
160160
options.update_shallow = 1;
161161
else if (!strcmp(value, "false"))
162162
options.update_shallow = 0;
163163
else
164164
return -1;
165165
return 0;
166-
} else if (!strcmp(name, "pushcert")) {
166+
} else if (!strncmp(name, "pushcert", namelen)) {
167167
if (!strcmp(value, "true"))
168168
options.push_cert = SEND_PACK_PUSH_CERT_ALWAYS;
169169
else if (!strcmp(value, "false"))
@@ -173,15 +173,15 @@ static int set_option(const char *name, const char *value)
173173
else
174174
return -1;
175175
return 0;
176-
} else if (!strcmp(name, "atomic")) {
176+
} else if (!strncmp(name, "atomic", namelen)) {
177177
if (!strcmp(value, "true"))
178178
options.atomic = 1;
179179
else if (!strcmp(value, "false"))
180180
options.atomic = 0;
181181
else
182182
return -1;
183183
return 0;
184-
} else if (!strcmp(name, "push-option")) {
184+
} else if (!strncmp(name, "push-option", namelen)) {
185185
if (*value != '"')
186186
string_list_append(&options.push_options, value);
187187
else {
@@ -192,7 +192,7 @@ static int set_option(const char *name, const char *value)
192192
strbuf_detach(&unquoted, NULL));
193193
}
194194
return 0;
195-
} else if (!strcmp(name, "family")) {
195+
} else if (!strncmp(name, "family", namelen)) {
196196
if (!strcmp(value, "ipv4"))
197197
git_curl_ipresolve = CURL_IPRESOLVE_V4;
198198
else if (!strcmp(value, "ipv6"))
@@ -202,16 +202,16 @@ static int set_option(const char *name, const char *value)
202202
else
203203
return -1;
204204
return 0;
205-
} else if (!strcmp(name, "from-promisor")) {
205+
} else if (!strncmp(name, "from-promisor", namelen)) {
206206
options.from_promisor = 1;
207207
return 0;
208-
} else if (!strcmp(name, "refetch")) {
208+
} else if (!strncmp(name, "refetch", namelen)) {
209209
options.refetch = 1;
210210
return 0;
211-
} else if (!strcmp(name, "filter")) {
211+
} else if (!strncmp(name, "filter", namelen)) {
212212
options.filter = xstrdup(value);
213213
return 0;
214-
} else if (!strcmp(name, "object-format")) {
214+
} else if (!strncmp(name, "object-format", namelen)) {
215215
options.object_format = 1;
216216
if (strcmp(value, "true"))
217217
die(_("unknown value for object-format: %s"), value);
@@ -1588,15 +1588,16 @@ int cmd_main(int argc, const char **argv)
15881588
parse_push(&buf);
15891589

15901590
} else if (skip_prefix(buf.buf, "option ", &arg)) {
1591-
char *value = strchr(arg, ' ');
1591+
const char *value = strchrnul(arg, ' ');
1592+
size_t arglen = value - arg;
15921593
int result;
15931594

1594-
if (value)
1595-
*value++ = '\0';
1595+
if (*value)
1596+
value++; /* skip over SP */
15961597
else
15971598
value = "true";
15981599

1599-
result = set_option(arg, value);
1600+
result = set_option(arg, arglen, value);
16001601
if (!result)
16011602
printf("ok\n");
16021603
else if (result < 0)

0 commit comments

Comments
 (0)