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

Re: [PATCH 4/5] ref_transaction_commit(): remove the local flags variables

From
Jeff King <peff@peff.net>
Date
Apr 24, 2015, 21:19 UTC
Message-ID
<20150424211932.GA10809@peff.net>
In-Reply-To
<553AB25D.9090201@alum.mit.edu>
On Fri, Apr 24, 2015 at 11:15:09PM +0200, Michael Haggerty wrote:
Show 17 quoted lines
> > Hmm. I think this is losing the distinction of "flags the caller has
> > passed in to us" versus "flags we are using locally only during the
> > transaction_commit routine". If callers look at the flags in the
> > REF_TRANSACTION_CLOSED state, do they care about seeing these new flags?
> > 
> > My guess is probably not in practice, and "leaking" these flags is an
> > acceptable tradeoff for keeping the transaction_commit function simpler.
> > But I haven't looked that closely.
> 
> "struct ref_update" is opaque to callers outside of the refs module, and
> ref_update::flags is not read anywhere outside of
> ref_transaction_commit() (and its value is passed to
> lock_ref_sha1_basic()). So I don't think we have to be shy about storing
> our own internal information there.
> 
> In fact, REF_DELETING, REF_ISPRUNING, REF_HAVE_NEW, and REF_HAVE_OLD are
> also private to the refs module.

Thanks for checking. If nobody is affected (and is not likely to be), I agree it's not worth worrying about.

Show 5 quoted lines
> I suppose we could mask out all the "private" bits in the flags
> parameter passed by the caller, to make sure that the caller hasn't
> accidentally set other bits. I think that would be more defensive than
> our usual practice, but I don't mind doing it if people think it would
> be prudent.
I don't think it's necessary.
-Peff
Previous: Michael HaggertyNext: Eric Sunshine
Message 8 of 19 in “Avoid file descriptor exhaustion in ref_transaction_commit()”
  1. 0/5 Avoid file descriptor exhaustion in ref_transaction_commit()Michael Haggerty, Apr 24, 2015
  2. 1/5 write_ref_to_lockfile(): new function, extracted from write_ref_sha1()Michael Haggerty, Apr 24, 2015
  3. 2/5 commit_ref_update(): new function, extracted from write_ref_sha1()Michael Haggerty, Apr 24, 2015
  4. 3/5 write_ref_sha1(): inline function at callersMichael Haggerty, Apr 24, 2015
  5. 4/5 ref_transaction_commit(): remove the local flags variablesMichael Haggerty, Apr 24, 2015
  6. Jeff KingApr 24, 2015
  7. Michael HaggertyApr 24, 2015
  8. Jeff KingApr 24, 2015
  9. Eric SunshineApr 24, 2015
  10. Michael HaggertyApr 24, 2015
  11. 5/5 ref_transaction_commit(): only keep one lockfile open at a timeMichael Haggerty, Apr 24, 2015
  12. Michael HaggertyApr 25, 2015
  13. Junio C HamanoApr 25, 2015
  14. Junio C HamanoApr 25, 2015
  15. Jeff KingApr 28, 2015
  16. Jeff KingApr 24, 2015
  17. Stefan BellerApr 24, 2015
  18. Junio C HamanoApr 25, 2015
  19. Junio C HamanoApr 25, 2015

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.