Skip to content

Commit 4eaed7c

Browse files
peffgitster
authored andcommitted
list-objects-filter: initialize sub-filter structs
Since commit c54980a (list-objects-filter: convert filter_spec to a strbuf, 2022-09-11), building with SANITIZE=undefined triggers an error in t5616. The problem is that we end up with a strbuf that has been zero-initialized instead of via STRBUF_INIT. Feeding that strbuf to strbuf_addbuf() in list_objects_filter_copy() means we will call memcpy like: memcpy(some_actual_buffer, NULL, 0); This works on most systems because we're copying zero bytes, but it is technically undefined behavior to ever pass NULL to memcpy. Even though c54980a is where the bug manifests, that is only because we switched away from a string_list, which is OK with being zero-initialized (though it may cause other problems by not duplicating the strings, it happened to be OK in this instance). The actual bug is caused by the commit before that, 2a01bde (list-objects-filter: add and use initializers, 2022-09-11). There we consistently initialize the top-level filter structs, but we forgot the dynamically allocated ones we stick in filter_options->sub when creating combined filters. Note that we need to fix two spots here: where we parse a "combine:" filter, but also where we transform from a single-filter into a combined one after seeing multiple "--filter" options. In the second spot, we'll do some minor refactoring to avoid repeating our very-long array index. Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent c54980a commit 4eaed7c

File tree

1 file changed

+7
-3
lines changed

1 file changed

+7
-3
lines changed

list-objects-filter-options.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ static int parse_combine_subfilter(
143143

144144
ALLOC_GROW_BY(filter_options->sub, filter_options->sub_nr, 1,
145145
filter_options->sub_alloc);
146+
list_objects_filter_init(&filter_options->sub[new_index]);
146147

147148
decoded = url_percent_decode(subspec->buf);
148149

@@ -263,6 +264,8 @@ void parse_list_objects_filter(
263264
parse_error = gently_parse_list_objects_filter(
264265
filter_options, arg, &errbuf);
265266
} else {
267+
struct list_objects_filter_options *sub;
268+
266269
/*
267270
* Make filter_options an LOFC_COMBINE spec so we can trivially
268271
* add subspecs to it.
@@ -273,10 +276,11 @@ void parse_list_objects_filter(
273276
filter_spec_append_urlencode(filter_options, arg);
274277
ALLOC_GROW_BY(filter_options->sub, filter_options->sub_nr, 1,
275278
filter_options->sub_alloc);
279+
sub = &filter_options->sub[filter_options->sub_nr - 1];
276280

277-
parse_error = gently_parse_list_objects_filter(
278-
&filter_options->sub[filter_options->sub_nr - 1], arg,
279-
&errbuf);
281+
list_objects_filter_init(sub);
282+
parse_error = gently_parse_list_objects_filter(sub, arg,
283+
&errbuf);
280284
}
281285
if (parse_error)
282286
die("%s", errbuf.buf);

0 commit comments

Comments
 (0)