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

Re: [PATCH 2/5] object.c: lookup_unknown_object() accept 'r' as parameter

From
Taylor Blau <me@ttaylorr.com>
Date
Feb 13, 2020, 18:00 UTC
Message-ID
<20200213180030.GF45518@syl.local>
In-Reply-To
<xmqqtv3vtst5.fsf@gitster-ct.c.googlers.com>
On Wed, Feb 12, 2020 at 01:11:02PM -0800, Junio C Hamano wrote:
Show 26 quoted lines
> Taylor Blau <me@ttaylorr.com> writes:
>
> > ... same question about why assigning:
> >
> >   struct repository *r = the_repository;
> >
> > and passing 'r' everywhere is preferable to simply passing
> > 'the_repository' in directly.
> > ...
> >>  static void mark_object_for_connectivity(const struct object_id *oid)
> >>  {
> >> -	struct object *obj = lookup_unknown_object(oid);
> >> +	struct repository *r = the_repository;
> >> +	struct object *obj = lookup_unknown_object(r, oid);
> >>  	obj->flags |= HAS_OBJ;
> >>  }
>
> I do not claim that it applies to this particular function, and the
> function is too small for it to matter, but when a function is large
> enough and it always works on one single repository, it would make
> it easier to later update the function further to set up 'r'
> upfront, making it point at the_repository for now, and to use 'r'
> throughout the function.  That way, when the time comes to update
> the function to work on an arbitrary repository, the only change
> required will be to turn the local variable 'r' to an incoming
> parameter the caller can supply.

Right, but my suggestion was that this advice doesn't apply to this particular instance since I don't expect that we'd ever passing something other than 'the_repository'.

Specifically, I was worried that we'd get bitten by re-assigning 'r' in the middle of the function and then end up in some odd broken state.

Maybe I'm worrying too much.

Thanks, Taylor

Previous: Junio C HamanoNext: Junio C Hamano
Message 11 of 17 in “object.c: localize global the_repository variable into r”
  1. 0/5 object.c: localize global the_repository variable into rParth Gala via GitGitGadget, Feb 12, 2020
  2. 3/5 object.c: parse_object_or_die() accept 'r' as parameterParth Gala via GitGitGadget, Feb 12, 2020
  3. 1/5 object.c: get_max_object_index and get_indexed_object accept 'r' parameterParth Gala via GitGitGadget, Feb 12, 2020
  4. Taylor BlauFeb 12, 2020
  5. Eric SunshineFeb 12, 2020
  6. parth galaFeb 13, 2020
  7. 4/5 object.c: clear_object_flags() accept 'r' as parameterParth Gala via GitGitGadget, Feb 12, 2020
  8. 2/5 object.c: lookup_unknown_object() accept 'r' as parameterParth Gala via GitGitGadget, Feb 12, 2020
  9. Taylor BlauFeb 12, 2020
  10. Junio C HamanoFeb 12, 2020
  11. Taylor BlauFeb 13, 2020
  12. Junio C HamanoFeb 13, 2020
  13. Jeff KingFeb 13, 2020
  14. Taylor BlauFeb 15, 2020
  15. 5/5 object.c: clear_commit_marks_all() accept 'r' as parameterParth Gala via GitGitGadget, Feb 12, 2020
  16. Taylor BlauFeb 12, 2020
  17. parth galaFeb 13, 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.