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

Re: [PATCH 5/5] shallow: use struct 'shallow_lock' for additional safety

From
Taylor Blau <me@ttaylorr.com>
Date
Apr 29, 2020, 23:51 UTC
Message-ID
<20200429235146.GA20588@syl.local>
In-Reply-To
<CAPig+cTvX1wT8kAha22uUQ5PDQ0mjeoDGQdA6VtMzfnAELpTaA@mail.gmail.com>
On Wed, Apr 29, 2020 at 07:03:48PM -0400, Eric Sunshine wrote:
Show 25 quoted lines
> On Wed, Apr 29, 2020 at 6:39 PM Taylor Blau <me@ttaylorr.com> wrote:
> > [...]
> > This patch introduces that type as a thin wrapper around 'struct
> > lockfile', and updates the two aforementioned functions and their
> > callers to use it.
> >
> > Signed-off-by: Taylor Blau <me@ttaylorr.com>
> > ---
> > diff --git a/shallow.h b/shallow.h
> > @@ -10,12 +10,22 @@ void set_alternate_shallow_file(struct repository *r, const char *path, int over
> > +/*
> > + * shallow_lock is a thin wrapper around 'struct lock_file' in order to restrict
> > + * which locks can be used with '{commit,rollback}_shallow_file()'.
> > + */
> > +struct shallow_lock {
> > +       struct lock_file lk;
> > +};
>
> The documentation comment for 'shallow_lock' may help newcomers to C
> but probably doesn't add much value for seasoned programmers. If this
> is the sort of idiom we want to introduce (or exists already in this
> codebase) -- declaring a specific C type to avoid accidental use of an
> unrelated lock -- then it's probably better documented in
> CodingGuidelines rather than repeating it at each point in the code
> which employs the idiom.

Fair enough; I definitely weighed the usefulness of this comment a little bit when I was writing it. I figured that I didn't want to commit to updating CodingGuidelines in this series, but that I didn't want to leave the explanation only in the commit message.

I figure that it's probably fine to document it in the commit message, and leave it at that for now.

Thanks for this suggestion (and the earlier ones, too). I have applied them locally, and I'll sit on them for a day or two before sending out v2. Thanks again.

Thanks, Taylor

Previous: Eric SunshineNext: Jonathan Tan
Message 18 of 22 in “shallow: extract a header file”
  1. 0/5 shallow: extract a header fileTaylor Blau, Apr 29, 2020
  2. 1/5 commit: make 'commit_graft_pos' non-staticTaylor Blau, Apr 29, 2020
  3. Jonathan NiederApr 30, 2020
  4. Taylor BlauApr 30, 2020
  5. 2/5 shallow: extract a header file for shallow-related functionsTaylor Blau, Apr 29, 2020
  6. Eric SunshineApr 29, 2020
  7. Jonathan TanApr 30, 2020
  8. Taylor BlauApr 30, 2020
  9. Jonathan TanApr 30, 2020
  10. Jonathan NiederApr 30, 2020
  11. 3/5 commit: move 'unregister_shallow' to 'shallow.h'Taylor Blau, Apr 29, 2020
  12. Jonathan NiederApr 30, 2020
  13. Taylor BlauApr 30, 2020
  14. 4/5 shallow.h: document '{commit,rollback}_shallow_file'Taylor Blau, Apr 29, 2020
  15. Eric SunshineApr 29, 2020
  16. 5/5 shallow: use struct 'shallow_lock' for additional safetyTaylor Blau, Apr 29, 2020
  17. Eric SunshineApr 29, 2020
  18. Taylor BlauApr 29, 2020
  19. Jonathan TanApr 30, 2020
  20. Jonathan NiederApr 30, 2020
  21. Eric SunshineApr 30, 2020
  22. Taylor BlauApr 30, 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.