Re: [PATCH 2/5] ieot: default to not writing IEOT section

From: Stefan Beller <sbeller@google.com>
Date: Nov 26, 2018, 22:02 UTC
Message-ID: <CAGZ79kZLFGtKzQLAXcrXRy8D9NhMwG=7iTCMzYu_h3Ba4G8cYQ@mail.gmail.com>
In-Reply-To: <7e06ec44-a3a0-fa38-75a7-7b875ae0679e@gmail.com>

On Mon, Nov 26, 2018 at 1:48 PM Ben Peart <peartben@gmail.com> wrote:

Show 24 quoted lines

>
>
>
> On 11/26/2018 2:59 PM, Stefan Beller wrote:
> >>> +static int record_ieot(void)
> >>> +{
> >>> +     int val;
> >>> +
> >>
> >> Initialize stack val to zero to ensure proper default.
> >
> > I don't think that is needed here, as we only use `val` when
> > we first write to it via git_config_get_bool.
> >
> > Did you spot this via code review and thought of
> > defensive programming or is there a tool that
> > has a false positive here?
> >
>
> Code review and defensive programming.  I had to review the code in
> git_config_get_bool() to see if it always initialized the val even if it
> didn't find the requested config variable (esp since we don't pass in a
> default value for this function like we do others).
>

Ah, sorry to have sent out this email, which I found as one of the earliest discussions in my mailbox. The later patches/discussions became a lot more heated from my cursory skimming and sorted out this as well.

It is interesting to notice that, as I also had to lookup how the config machinery works (once? a couple times?) but now it is so hardcoded in my brain to assume that if functions like git_config_* take the branch, we can access the value that the config function was supposed to read into.

Sorry for the noise, Stefan