@matheusbsbs / matheus-bagliano / commits / 4f33a6345f

list-objects: handle NULL function pointers

If a caller to traverse_commit_list() specifies the options for the --objects flag but does not specify a show_object function pointer, the result is a segfault. This is currently visible by running 'git bundle create --objects HEAD'. We could fix this problem by supplying a no-op callback in builtin/bundle.c, but that only solves the problem for one builtin, leaving this segfault open for other callers. Replace all callers of the show_commit and show_object function pointers in list-objects.c to call helper functions show_commit() and show_object() which check that the given context has non-NULL functions before passing the necessary data. One extra benefit is that it reduces duplication due to passing ctx->show_data to every caller. Test that this segfault no longer occurs for 'git bundle'. Co-authored-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Mar 9, 2022 at 16:01 UTC 4f33a6345f2bd6e47253d1dbefd01874d895ab2e
3 files changed +36 -5
bundle.c
+2
@@ -544,6 +544,8 @@ int create_bundle(struct repository *r, const char *path,
544 die("revision walk setup failed");
545 bpi.fd = bundle_fd;
546 bpi.pending = &revs_copy.pending;
547 +
548 + revs.blob_objects = revs.tree_objects = 0;
549 traverse_commit_list(&revs, write_bundle_prerequisites, NULL, &bpi);
550 object_array_remove_duplicates(&revs_copy.pending);
551
list-objects.c
+22 -5
@@ -21,6 +21,23 @@ struct traversal_context {
21 struct filter *filter;
22 };
23
24 +static void show_commit(struct traversal_context *ctx,
25 + struct commit *commit)
26 +{
27 + if (!ctx->show_commit)
28 + return;
29 + ctx->show_commit(commit, ctx->show_data);
30 +}
31 +
32 +static void show_object(struct traversal_context *ctx,
33 + struct object *object,
34 + const char *name)
35 +{
36 + if (!ctx->show_object)
37 + return;
38 + ctx->show_object(object, name, ctx->show_data);
39 +}
40 +
41 static void process_blob(struct traversal_context *ctx,
42 struct blob *blob,
43 struct strbuf *path,
@@ -60,7 +77,7 @@ static void process_blob(struct traversal_context *ctx,
77 if (r & LOFR_MARK_SEEN)
78 obj->flags |= SEEN;
79 if (r & LOFR_DO_SHOW)
63 - ctx->show_object(obj, path->buf, ctx->show_data);
80 + show_object(ctx, obj, path->buf);
81 strbuf_setlen(path, pathlen);
82 }
83
@@ -194,7 +211,7 @@ static void process_tree(struct traversal_context *ctx,
211 if (r & LOFR_MARK_SEEN)
212 obj->flags |= SEEN;
213 if (r & LOFR_DO_SHOW)
197 - ctx->show_object(obj, base->buf, ctx->show_data);
214 + show_object(ctx, obj, base->buf);
215 if (base->len)
216 strbuf_addch(base, '/');
217
@@ -210,7 +227,7 @@ static void process_tree(struct traversal_context *ctx,
227 if (r & LOFR_MARK_SEEN)
228 obj->flags |= SEEN;
229 if (r & LOFR_DO_SHOW)
213 - ctx->show_object(obj, base->buf, ctx->show_data);
230 + show_object(ctx, obj, base->buf);
231
232 strbuf_setlen(base, baselen);
233 free_tree_buffer(tree);
@@ -228,7 +245,7 @@ static void process_tag(struct traversal_context *ctx,
245 if (r & LOFR_MARK_SEEN)
246 tag->object.flags |= SEEN;
247 if (r & LOFR_DO_SHOW)
231 - ctx->show_object(&tag->object, name, ctx->show_data);
248 + show_object(ctx, &tag->object, name);
249 }
250
251 static void mark_edge_parents_uninteresting(struct commit *commit,
@@ -402,7 +419,7 @@ static void do_traverse(struct traversal_context *ctx)
419 if (r & LOFR_MARK_SEEN)
420 commit->object.flags |= SEEN;
421 if (r & LOFR_DO_SHOW)
405 - ctx->show_commit(commit, ctx->show_data);
422 + show_commit(ctx, commit);
423
424 if (ctx->revs->tree_blobs_in_commit_order)
425 /*
t/t6020-bundle-misc.sh
+12
@@ -475,4 +475,16 @@ test_expect_success 'clone from bundle' '
475 test_cmp expect actual
476 '
477
478 +test_expect_success 'unfiltered bundle with --objects' '
479 + git bundle create all-objects.bdl \
480 + --all --objects &&
481 + git bundle create all.bdl \
482 + --all &&
483 +
484 + # Compare the headers of these files.
485 + sed -n -e "/^$/q" -e "p" all.bdl >expect &&
486 + sed -n -e "/^$/q" -e "p" all-objects.bdl >actual &&
487 + test_cmp expect actual
488 +'
489 +
490 test_done