Re: [PATCH 1/5] eoie: default to not writing EOIE section

From: Jeff King <peff@peff.net>
Date: Nov 21, 2018, 16:46 UTC
Message-ID: <20181121164619.GA13860@sigill.intra.peff.net>
In-Reply-To: <20181120132151.GA30222@szeder.dev>

On Tue, Nov 20, 2018 at 02:21:51PM +0100, SZEDER Gábor wrote:

Show 16 quoted lines

> On Tue, Nov 20, 2018 at 08:06:16AM -0500, Ben Peart wrote:
> > >diff --git a/read-cache.c b/read-cache.c
> > >index 4ca81286c0..1e9c772603 100644
> > >--- a/read-cache.c
> > >+++ b/read-cache.c
> > >@@ -2689,6 +2689,15 @@ void update_index_if_able(struct index_state *istate, struct lock_file *lockfile
> > >  		rollback_lock_file(lockfile);
> > >  }
> > >+static int record_eoie(void)
> > >+{
> > >+	int val;
> > 
> > I believe you are going to want to initialize val to 0 here as it is on the
> > stack so is not guaranteed to be zero.
> 
> The git_config_get_bool() call below will initialize it anyway.

Yes, there are two ways to write this. With a conditional to initialize and return or to return the default, as we have here:

> > >+	if (!git_config_get_bool("index.recordendofindexentries", &val))
> > >+		return val;
> > >+	return 0;

Or initialize the default ahead of time, and rely on the function not to modify it when the entry is missing:

  int val = 0;
  git_config_get_bool("index.whatever", &val);
  return val;

I think either is perfectly fine, but since I also had to look at it twice to make sure it was doing the right thing, I figured it is worth mentioning as a possible style/convention thing we may want to decide on.

-Peff