From: Duy Nguyen <firstname.lastname@example.org> To: Junio C Hamano <email@example.com> Cc: Jeff King <firstname.lastname@example.org>, Eric Wong <email@example.com>, Antonio Ospite <firstname.lastname@example.org>, Git Mailing List <email@example.com> Subject: Re: [PATCH] get_oid: handle NULL repo->index Date: Wed, 15 May 2019 16:29:12 +0700 [thread overview] Message-ID: <CACsJy8BS8NR6aZR29VLYUrRjBE_oyzH=L6X7CSpCO9G+sPjcbA@mail.gmail.com> (raw) In-Reply-To: <firstname.lastname@example.org> On Wed, May 15, 2019 at 12:16 PM Junio C Hamano <email@example.com> wrote: > > Jeff King <firstname.lastname@example.org> writes: > > > Also from my earlier message, if you missed it: > > > > I also wondered if we should simply allocate an empty index whenever > > we have a non-toplevel "struct repository", which might be less > > surprising to other callers. I don't have a strong opinion either way. > > I did grep around for other callers which might have similar problems, > > but couldn't find any. > > That is an approach to make it harder to make mistakes by accepting > possibly a small wasted resource; but at that point, I think calling > repo_read_index() unconditionally from here and similar places would > be a simpler fix in the same spirit. For repo_read_index() case, maybe. But we have a lot of "r(epo)->index->something". All or most of these references traditionally are the_index.something, which is always safe to dereference. Submodule repos with the optionally NULL repo->index break this assumption. So either we audit all the code and make sure "repo->index" cannot be NULL because repo_read_index() has been called before (and may have to repeat auditing), or we just stick to the old assumption and make sure repo->index is not NULL from the beginning. This makes me think the small extra resource is worth it. Much less time will be spent on similar issues now and in the future. PS. Sorry Jeff for the noise. I should have waited until I come home and can read your mail more carefully. -- Duy
next prev parent reply other threads:[~2019-05-15 9:29 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-05-11 20:57 new segfault in master (6a6c0f10a70a6eb1) Eric Wong 2019-05-11 22:31 ` Jeff King 2019-05-11 23:02 ` Jeff King 2019-05-12 4:26 ` Duy Nguyen 2019-05-14 13:54 ` [PATCH] get_oid: handle NULL repo->index Jeff King 2019-05-14 23:38 ` Eric Wong 2019-05-15 1:24 ` Duy Nguyen 2019-05-15 1:46 ` Jeff King 2019-05-15 5:16 ` Junio C Hamano 2019-05-15 9:29 ` Duy Nguyen [this message] 2019-05-16 1:43 ` Junio C Hamano 2019-05-19 2:56 ` [PATCH] repository.c: always allocate 'index' at repo init time Nguyễn Thái Ngọc Duy 2019-05-20 13:17 ` Jeff King 2019-05-21 10:34 ` Duy Nguyen 2019-05-21 20:58 ` Jeff King 2019-05-28 16:07 ` Junio C Hamano
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style List information: http://vger.kernel.org/majordomo-info.html * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CACsJy8BS8NR6aZR29VLYUrRjBE_oyzH=L6X7CSpCO9G+sPjcbA@mail.gmail.com' \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --subject='Re: [PATCH] get_oid: handle NULL repo->index' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Code repositories for project(s) associated with this inbox: https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).