On Wed, Aug 18, 2021 at 12:38 AM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > > > From: Han-Wen Nienhuys <hanwen@xxxxxxxxxx> > > > > This fixes a crash triggered by the BUG() statement. > > > > This can occur with symlinked .git/refs. To check availability, > > refs_verify_refname_available will run refs_read_raw_ref() on each prefix, > > leading to a read() from .git/refs (which is a directory). > > > > When handling the symlink case, it is probably more robust to re-issue the > > lstat() as a normal stat(), in which case, we would fall back to the directory > > case. > > > > For now, propagating errno from strbuf_read() is simpler and avoids the crash. > > > > Signed-off-by: Han-Wen Nienhuys <hanwen@xxxxxxxxxx> > > --- > > fixup! propagate errno from failing read > > Hmph, I do not see a commit with "propagate errno from failing read" > in its title anywhere in 'seen'. > I think the convention to assign errno to myerr in this codepath > originates in a0731250 (refs: explicitly return failure_errno from > parse_loose_ref_contents, 2021-07-20), and it forgot the part of the > code being fixed with this patch. The commit being fixed is already > is in 'next' as part of the hn/refs-errno-cleanup topic. > > Usually, a flaw in a topic that is already in 'next' is corrected by > a follow-up patch, but then they won't say "fixup!" (none of our > bugfix patches do). But a post-release is a special time, as we > will soon be rewinding 'next', restarting it from the latest release > and we have a choice to tentatively eject a topic, fix it up or > even replace it, before merging the corrected topic to 'next'. > > Do you mean that you want me to squash this change into that commit > before the topic graduates to 'master' during the new development > cycle? If so please be a bit more explicit next time. Using the > title of the commit after "fixup!" would be a good starting point. The problem fixed here affects anyone who uses git-repo (ie. does Android development) and runs "git-branch -m", which is a large group of people, so I think it should not be allowed to get into a release. So it could be squashed into commit a0731250, or put on top of next as a separate commit (probably with 'fixup!' removed). Note that, even though commit a0731250 originates from a branch called "hn/XXX" and has me as Author, the BUG() call causing the crash was actually introduced by AEvar when he reworked the series. -- Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
