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