On 2020-05-19 at 23:42:08, brian m. carlson wrote:
> On 2020-05-19 at 12:11:15, Derrick Stolee wrote:
> > On 5/15/2020 1:22 PM, Derrick Stolee wrote:
> > > brian m. carlson 13e7ed6a builtin/checkout: compute checkout metadata for checkouts
> > > builtin/checkout.c
> > > 13e7ed6a 625) is_null_oid(&info->oid) ? &tree->object.oid :
> >
> > This is part of the following hunk:
> >
> > @@ -619,6 +620,11 @@ static int reset_tree(struct tree *tree, const struct checkout_opts *o,
> > opts.verbose_update = o->show_progress;
> > opts.src_index = &the_index;
> > opts.dst_index = &the_index;
> > + init_checkout_metadata(&opts.meta, info->refname,
> > + info->commit ? &info->commit->object.oid :
> > + is_null_oid(&info->oid) ? &tree->object.oid :
> > + &info->oid,
> > + NULL);
> > parse_tree(tree);
> > init_tree_desc(&tree_desc, tree->buffer, tree->size);
> > switch (unpack_trees(1, &tree_desc, &opts)) {
> >
> > The double-nested ternary definitely complicates the coverage here.
> > It also points out that all tests have `info->commit` a non-NULL.
> >
> > This certainly looks safe, but I don't know.
>
> This is me trying to be defensive. I think the code path that is not
> covered here that can be is the info->oid code path; I don't technically
> believe the other case (using the tree) is possible, although the
> checkout code is complex enough that I can't be certain.
Actually, with more research, it looks like this can just be simplified
since info->commit must be non-NULL. I've analyzed the code and am
confident that's guaranteed to be the case, and the test suite has
confirmed no segfaults by making that assumption, so I'll send a patch
imminently that just does that.
--
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204
Attachment:
signature.asc
Description: PGP signature
