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

Re: [PATCH v2 2/4] config: drop file pointer validity check in get_next_char()

From
Jeff King <peff@peff.net>
Date
Mar 12, 2013, 19:18 UTC
Message-ID
<20130312191809.GD17099@sigill.intra.peff.net>
In-Reply-To
<20130312160056.GB4472@sandbox-ub.fritz.box>
On Tue, Mar 12, 2013 at 05:00:56PM +0100, Heiko Voigt wrote:
Show 11 quoted lines
> > That is, every path to get_next_char happens while we are in
> > git_config_from_file, and that function guarantees that cf = &top, and
> > that top.f != NULL.  We do not have to even do any analysis of the
> > conditions for each call, because we never change "cf" nor "top.f"
> > except when we set them in git_config_from_file.
> 
> Ok if you say so I will do that :-). I was thinking about adding a patch
> that would remove cf as a global variable and explicitely pass it down
> to get_next_char. That makes it more obvious that it actually is != NULL.
> Looking at your callgraph I do not think its that much work. What do you
> think?

Yeah, I think that makes it more obvious what is going on, but you will run into trouble if you ever want that information to cross the "void *" boundary of a config callback, as adding a new parameter there is hard.

The only place we do that now is for git_config_include, and I think you could get by with stuffing it into the config_include_data struct (which is already there to deal with this problem).

It would also make something like this patch hard or impossible:
  http://article.gmane.org/gmane.comp.version-control.git/190267

as it wants to access "cf" from arbitrary callbacks. That is not a patch that is actively under consideration, but I had considered polishing it up at some point.

> BTW, how did you generate this callgraph? Do you have a nice tool for that?

I just did it manually in vi, working backwards from every instance of get_next_char, as it is not that big a graph. I suspect something like cscope could make it easier, but I don't know of any tool that will build the graph automatically (it would not be that hard from the data cscope has, though, so I wouldn't be surprised if such a tool exists).

-Peff
Previous: Jeff KingNext: Heiko Voigt
Message 12 of 24 in “allow more sources for config values”
  1. 0/4 allow more sources for config valuesHeiko Voigt, Mar 10, 2013
  2. 1/4 config: factor out config file stack managementHeiko Voigt, Mar 10, 2013
  3. Jeff KingMar 12, 2013
  4. Heiko VoigtMar 12, 2013
  5. Jeff KingMar 12, 2013
  6. Heiko VoigtMar 14, 2013
  7. 2/4 config: drop file pointer validity check in get_next_char()Heiko Voigt, Mar 10, 2013
  8. Jeff KingMar 12, 2013
  9. Heiko VoigtMar 12, 2013
  10. Heiko VoigtMar 12, 2013
  11. Jeff KingMar 12, 2013
  12. Jeff KingMar 12, 2013
  13. 3/4 config: make parsing stack struct independent from actual data sourceHeiko Voigt, Mar 10, 2013
  14. Jeff KingMar 12, 2013
  15. Heiko VoigtMar 12, 2013
  16. Jeff KingMar 12, 2013
  17. 4/4 teach config parsing to read from strbufHeiko Voigt, Mar 10, 2013
  18. Jeff KingMar 12, 2013
  19. Heiko VoigtMar 12, 2013
  20. Jeff KingMar 12, 2013
  21. Heiko VoigtMar 14, 2013
  22. Jeff KingMar 14, 2013
  23. Jeff KingMar 14, 2013
  24. Heiko VoigtMar 18, 2013

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.