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 <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Jun 7, 2024 at 08:39 UTC a3da6948c3b6c272bf7a381b86df9a60bdf2b052
1 file changed +27 -26
remote-curl.c
+27 -26
@@ -58,9 +58,9 @@ struct options {
58 static struct options options;
59 static struct string_list cas_options = STRING_LIST_INIT_DUP;
60
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)
62 {
63 - if (!strcmp(name, "verbosity")) {
63 + if (!strncmp(name, "verbosity", namelen)) {
64 char *end;
65 int v = strtol(value, &end, 10);
66 if (value == end || *end)
@@ -68,7 +68,7 @@ static int set_option(const char *name, const char *value)
68 options.verbosity = v;
69 return 0;
70 }
71 - else if (!strcmp(name, "progress")) {
71 + else if (!strncmp(name, "progress", namelen)) {
72 if (!strcmp(value, "true"))
73 options.progress = 1;
74 else if (!strcmp(value, "false"))
@@ -77,7 +77,7 @@ static int set_option(const char *name, const char *value)
77 return -1;
78 return 0;
79 }
80 - else if (!strcmp(name, "depth")) {
80 + else if (!strncmp(name, "depth", namelen)) {
81 char *end;
82 unsigned long v = strtoul(value, &end, 10);
83 if (value == end || *end)
@@ -85,15 +85,15 @@ static int set_option(const char *name, const char *value)
85 options.depth = v;
86 return 0;
87 }
88 - else if (!strcmp(name, "deepen-since")) {
88 + else if (!strncmp(name, "deepen-since", namelen)) {
89 options.deepen_since = xstrdup(value);
90 return 0;
91 }
92 - else if (!strcmp(name, "deepen-not")) {
92 + else if (!strncmp(name, "deepen-not", namelen)) {
93 string_list_append(&options.deepen_not, value);
94 return 0;
95 }
96 - else if (!strcmp(name, "deepen-relative")) {
96 + else if (!strncmp(name, "deepen-relative", namelen)) {
97 if (!strcmp(value, "true"))
98 options.deepen_relative = 1;
99 else if (!strcmp(value, "false"))
@@ -102,7 +102,7 @@ static int set_option(const char *name, const char *value)
102 return -1;
103 return 0;
104 }
105 - else if (!strcmp(name, "followtags")) {
105 + else if (!strncmp(name, "followtags", namelen)) {
106 if (!strcmp(value, "true"))
107 options.followtags = 1;
108 else if (!strcmp(value, "false"))
@@ -111,7 +111,7 @@ static int set_option(const char *name, const char *value)
111 return -1;
112 return 0;
113 }
114 - else if (!strcmp(name, "dry-run")) {
114 + else if (!strncmp(name, "dry-run", namelen)) {
115 if (!strcmp(value, "true"))
116 options.dry_run = 1;
117 else if (!strcmp(value, "false"))
@@ -120,7 +120,7 @@ static int set_option(const char *name, const char *value)
120 return -1;
121 return 0;
122 }
123 - else if (!strcmp(name, "check-connectivity")) {
123 + else if (!strncmp(name, "check-connectivity", namelen)) {
124 if (!strcmp(value, "true"))
125 options.check_self_contained_and_connected = 1;
126 else if (!strcmp(value, "false"))
@@ -129,7 +129,7 @@ static int set_option(const char *name, const char *value)
129 return -1;
130 return 0;
131 }
132 - else if (!strcmp(name, "cas")) {
132 + else if (!strncmp(name, "cas", namelen)) {
133 struct strbuf val = STRBUF_INIT;
134 strbuf_addstr(&val, "--force-with-lease=");
135 if (*value != '"')
@@ -139,7 +139,7 @@ static int set_option(const char *name, const char *value)
139 string_list_append(&cas_options, val.buf);
140 strbuf_release(&val);
141 return 0;
142 - } else if (!strcmp(name, TRANS_OPT_FORCE_IF_INCLUDES)) {
142 + } else if (!strncmp(name, TRANS_OPT_FORCE_IF_INCLUDES, namelen)) {
143 if (!strcmp(value, "true"))
144 options.force_if_includes = 1;
145 else if (!strcmp(value, "false"))
@@ -147,7 +147,7 @@ static int set_option(const char *name, const char *value)
147 else
148 return -1;
149 return 0;
150 - } else if (!strcmp(name, "cloning")) {
150 + } else if (!strncmp(name, "cloning", namelen)) {
151 if (!strcmp(value, "true"))
152 options.cloning = 1;
153 else if (!strcmp(value, "false"))
@@ -155,7 +155,7 @@ static int set_option(const char *name, const char *value)
155 else
156 return -1;
157 return 0;
158 - } else if (!strcmp(name, "update-shallow")) {
158 + } else if (!strncmp(name, "update-shallow", namelen)) {
159 if (!strcmp(value, "true"))
160 options.update_shallow = 1;
161 else if (!strcmp(value, "false"))
@@ -163,7 +163,7 @@ static int set_option(const char *name, const char *value)
163 else
164 return -1;
165 return 0;
166 - } else if (!strcmp(name, "pushcert")) {
166 + } else if (!strncmp(name, "pushcert", namelen)) {
167 if (!strcmp(value, "true"))
168 options.push_cert = SEND_PACK_PUSH_CERT_ALWAYS;
169 else if (!strcmp(value, "false"))
@@ -173,7 +173,7 @@ static int set_option(const char *name, const char *value)
173 else
174 return -1;
175 return 0;
176 - } else if (!strcmp(name, "atomic")) {
176 + } else if (!strncmp(name, "atomic", namelen)) {
177 if (!strcmp(value, "true"))
178 options.atomic = 1;
179 else if (!strcmp(value, "false"))
@@ -181,7 +181,7 @@ static int set_option(const char *name, const char *value)
181 else
182 return -1;
183 return 0;
184 - } else if (!strcmp(name, "push-option")) {
184 + } else if (!strncmp(name, "push-option", namelen)) {
185 if (*value != '"')
186 string_list_append(&options.push_options, value);
187 else {
@@ -192,7 +192,7 @@ static int set_option(const char *name, const char *value)
192 strbuf_detach(&unquoted, NULL));
193 }
194 return 0;
195 - } else if (!strcmp(name, "family")) {
195 + } else if (!strncmp(name, "family", namelen)) {
196 if (!strcmp(value, "ipv4"))
197 git_curl_ipresolve = CURL_IPRESOLVE_V4;
198 else if (!strcmp(value, "ipv6"))
@@ -202,16 +202,16 @@ static int set_option(const char *name, const char *value)
202 else
203 return -1;
204 return 0;
205 - } else if (!strcmp(name, "from-promisor")) {
205 + } else if (!strncmp(name, "from-promisor", namelen)) {
206 options.from_promisor = 1;
207 return 0;
208 - } else if (!strcmp(name, "refetch")) {
208 + } else if (!strncmp(name, "refetch", namelen)) {
209 options.refetch = 1;
210 return 0;
211 - } else if (!strcmp(name, "filter")) {
211 + } else if (!strncmp(name, "filter", namelen)) {
212 options.filter = xstrdup(value);
213 return 0;
214 - } else if (!strcmp(name, "object-format")) {
214 + } else if (!strncmp(name, "object-format", namelen)) {
215 options.object_format = 1;
216 if (strcmp(value, "true"))
217 die(_("unknown value for object-format: %s"), value);
@@ -1588,15 +1588,16 @@ int cmd_main(int argc, const char **argv)
1588 parse_push(&buf);
1589
1590 } 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;
1593 int result;
1594
1594 - if (value)
1595 - *value++ = '\0';
1595 + if (*value)
1596 + value++; /* skip over SP */
1597 else
1598 value = "true";
1599
1599 - result = set_option(arg, value);
1600 + result = set_option(arg, arglen, value);
1601 if (!result)
1602 printf("ok\n");
1603 else if (result < 0)