git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: Git Test Coverage Report (v2.27.0-rc0)

From
brian m. carlson <sandals@crustytoothpaste.net>
Date
May 20, 2020, 01:38 UTC
Message-ID
<20200520013812.GS6362@camp.crustytoothpaste.net>
In-Reply-To
<20200519234208.GR6362@camp.crustytoothpaste.net>
On 2020-05-19 at 23:42:08, brian m. carlson wrote:
Show 30 quoted lines
> 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
Previous: brian m. carlsonNext: Jonathan Tan
Message 5 of 16 in “Git Test Coverage Report (v2.27.0-rc0)”
  1. Derrick StoleeMay 15, 2020
  2. Derrick StoleeMay 19, 2020
  3. René ScharfeMay 19, 2020
  4. brian m. carlsonMay 19, 2020
  5. brian m. carlsonMay 20, 2020
  6. t4067: make rename detection test output raw diffJonathan Tan, May 19, 2020
  7. Derrick StoleeMay 19, 2020
  8. 1/1 builtin/checkout: simplify metadata initializationbrian m. carlson, May 20, 2020
  9. Junio C HamanoMay 20, 2020
  10. brian m. carlsonMay 20, 2020
  11. 0/2 Improve Fix code coverage for checkoutbrian m. carlson, May 21, 2020
  12. 1/2 builtin/checkout: simplify metadata initializationbrian m. carlson, May 21, 2020
  13. Junio C HamanoMay 21, 2020
  14. brian m. carlsonMay 23, 2020
  15. 2/2 t2060: add a test for switch with --orphan and --discard-changesbrian m. carlson, May 21, 2020
  16. Derrick StoleeMay 21, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.