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

Re: [PATCH v2 1/2] builtin/checkout: simplify metadata initialization

From
brian m. carlson <sandals@crustytoothpaste.net>
Date
May 23, 2020, 12:22 UTC
Message-ID
<20200523122205.GA1915090@camp.crustytoothpaste.net>
In-Reply-To
<xmqq1rndtcmd.fsf@gitster.c.googlers.com>
On 2020-05-21 at 17:35:22, Junio C Hamano wrote:
Show 12 quoted lines
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> 
> > ...  The only case in which we do not have a commit object is when
> > invoking git switch with --orphan.  Moreover, we can only hit this code
> > path without a commit object additionally with either --force or
> > --discard-changes.
> 
> It was easy for me to trace the codepath to see when these options
> are used we end up with no commit object, but I ran out of time
> trying to see if the "forced orphan" is the only way to end up with
> a NULL in new_branch_info->commit.  Assuming that is true, of course
> the following perfectly makes sense.

I believe it is. The only case in which we have a NULL commit as far as I can tell is with switch and an orphan, and in merge_working_tree we call reset_tree either if the changes are discarded or unpack_trees couldn't do a trivial merge. Since I'm pretty sure unpack_trees can indeed merge with the empty tree, we would only call reset_trees with --discard-changes or --force. And reset_tree is only called from merge_working_tree.

In addition, I did try other situations plus the entire testsuite with my erroneous first patch and was unable to cause a segfault anywhere (which would have been a trivial NULL dereference) in case I missed something, which leads me to believe that this is in fact the only situation in which this occurs.

-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204
Previous: Junio C HamanoNext: brian m. carlson
Message 14 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.