Re: [PATCHv2 2/8] submodule config: keep update strategy around

From: Stefan Beller <sbeller@google.com>
Date: Oct 30, 2015, 18:25 UTC
Message-ID: <CAGZ79kYCmqv6vqRERWmihs5Ym-ug_xiqebMjQMDzjAmHgwKPGw@mail.gmail.com>
In-Reply-To: <CAPig+cRh9J0izFvLzRjjU4FEBKJsiJaYFv=9WdxFVfJ3xs0JiQ@mail.gmail.com>

On Fri, Oct 30, 2015 at 11:16 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:

Show 14 quoted lines

> On Fri, Oct 30, 2015 at 1:38 PM, Stefan Beller <sbeller@google.com> wrote:
>> On Thu, Oct 29, 2015 at 6:14 PM, Eric Sunshine <ericsunshine@gmail.com> wrote:
>>>> +               else if (!me->overwrite && submodule->update != NULL)
>>>
>>> Although "foo != NULL" is unusual in this code-base, it is used
>>> elsewhere in this file, including just outside the context seen above.
>>> Okay.
>>
>> ok, I'll clean that up as we go.
>
> Oh, I wasn't suggesting that you clean this up (though you may if you
> want). I was merely commenting (for the sake of others reviewing this
> patch) that, while not the norm for the project, this instance is
> consistent with surrounding code.

I only did a separate patch on top cleaning up 4 occurrences in that file. We use != NULL quite often throughout the code base, specially in conditions with side effects like:

    while ((char *c = string++) != NULL) {
        ...

where I think that makes even sense. But there are a minor number of cases where we have no side effects

    $ grep -rI "!= NULL" |grep -v "((" |grep -v "))" |wc -l
    135

Show 7 quoted lines

>
>>>> +                       free((void *)submodule->update);
>>>
>>> Minor: Every other 'free((void *) foo)' in this file has a space after
>>> "(void *)", one of which can be seen in the context just above.
>>
>> done