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

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

From
Heiko Voigt <hvoigt@hvoigt.net>
Date
Feb 28, 2013, 00:54 UTC
Message-ID
<20130228005432.GA23046@sandbox-ub>
In-Reply-To
<20130228004137.GA12948@sandbox-ub>
On Thu, Feb 28, 2013 at 01:41:47AM +0100, Heiko Voigt wrote:
Show 30 quoted lines
> On Wed, Feb 27, 2013 at 08:52:57AM +0100, Heiko Voigt wrote:
> > On Tue, Feb 26, 2013 at 03:05:56PM -0500, Jeff King wrote:
> > > On Tue, Feb 26, 2013 at 08:40:23PM +0100, Heiko Voigt wrote:
> > > 
> > > > The only location where cf is set in this file is in do_config_from().
> > > > This function has only one callsite which is config_from_file(). In
> > > > config_from_file() its ensured that the f member is set to non-zero.
> > > 
> > > Makes sense, although...
> > > 
> > > > -	if (cf && ((f = cf->f) != NULL)) {
> > > > +	if (cf) {
> > > > +		FILE *f = cf->f;
> > > 
> > > Couldn't we say the same thing about "cf" here (i.e., that it would
> > > never be NULL)? Can we just get rid of this conditional entirely?
> > 
> > That might be true. I will look into it. Just wanted to get rid of an
> > extra callback in my series.
> 
> I had a look and it might be true that cf will never be NULL in a code
> path. Nevertheless its much harder to verify by looking at the code
> since its a global variable. get_next_char() is called from all over the
> place and I would have to look at all the code paths. As far as I know
> static global variables are always initialized to zero so its safe to
> check even if has not yet been explicitly initialized.
> 
> The statement if cf is not NULL all members will be initialized is much
> simpler to verify since its just one place now and two places after this
> series.

To add some empirical information: I just ran the testsuite without the conditional and it still passes. To me it only make sense to start the parsing with cf initialized. But I am not familiar enough with the code to judge whether it is safe to assume this.

Cheers Heiko
Previous: Heiko VoigtNext: Heiko Voigt
Message 18 of 25 in “fetch moved submodules on-demand”
  1. 0/3 fetch moved submodules on-demandHeiko Voigt, Feb 25, 2013
  2. 1/3 teach config parsing to read from strbufHeiko Voigt, Feb 25, 2013
  3. Junio C HamanoFeb 25, 2013
  4. Heiko VoigtFeb 25, 2013
  5. 0/4 allow more sources for config valuesHeiko Voigt, Feb 26, 2013
  6. 1/4 config: factor out config file stack managementHeiko Voigt, Feb 26, 2013
  7. Jeff KingFeb 26, 2013
  8. Heiko VoigtFeb 26, 2013
  9. Jeff KingFeb 26, 2013
  10. Junio C HamanoFeb 26, 2013
  11. Heiko VoigtFeb 27, 2013
  12. Junio C HamanoFeb 26, 2013
  13. Heiko VoigtFeb 27, 2013
  14. 2/4 config: drop file pointer validity check in get_next_char()Heiko Voigt, Feb 26, 2013
  15. Jeff KingFeb 26, 2013
  16. Heiko VoigtFeb 27, 2013
  17. Heiko VoigtFeb 28, 2013
  18. Heiko VoigtFeb 28, 2013
  19. 3/4 config: make parsing stack struct independent from actual data sourceHeiko Voigt, Feb 26, 2013
  20. 4/4 teach config parsing to read from strbufHeiko Voigt, Feb 26, 2013
  21. Ramsay JonesMar 7, 2013
  22. Heiko VoigtMar 10, 2013
  23. Jeff KingFeb 26, 2013
  24. 2/3 implement fetching of moved submodulesHeiko Voigt, Feb 25, 2013
  25. 3/3 submodule: simplify decision tree whether to or not to fetchHeiko Voigt, Feb 25, 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.