Re: [PATCH 01/23] contrib/coccinnelle: add equals-null.cocci

From: Philip Oakley <philipoakley@iee.email>
Date: May 1, 2022, 17:22 UTC
Message-ID: <d996dc46-ae9a-6506-8043-0ac5468d9dd4@iee.email>
In-Reply-To: <CA+EOSBnx3-G02=zXGUrRuKPTDPBSYoBY=rERCORe8NtywEOiGg@mail.gmail.com>

On 01/05/2022 18:04, Elia Pinto wrote:

Show 15 quoted lines

> Il giorno dom 1 mag 2022 alle ore 02:20 Junio C Hamano
> <gitster@pobox.com> ha scritto:
>> What I found curious is that the result of applying these patches to
>> v2.36.0 and running coccicheck reveals that we are not making the
>> codebase clean wrt this new coccinelle rule.
>>
> It is possible, I did not use coccicheck to apply the semantic patch
> (on next)  but i use a my script which I think is slightly more
> efficient but perhaps it is not so correct. Anyway, given the
> discussion that has taken place so far, what do you think is best for
> me to do? Do a reroll (perhaps with only 2 patches in total ) or wait
> for the "right" moment in the future as foreseen by the Documentation
> and dedicate the time to more useful contributions for git? Thank you
> all for the review
>

Hi Elia,

Given Junio's comment regarding the potential for patch churn, it may, as an alternative, be possible to create a script that will check/extract just those those changes that are either:

A) mistakes in patches between say master and seen `master..seen` so that they aren't incorporated, though `next..seen` may be more appropriate.

B) detect 'while you are at it' fixes that could be applied to a file that is being modified with the same range query. The file fix-up patch could/would then be inserted as preparatory step to the relevant series (i.e. fed to the series author), allowing easy reversion with/without the fix-up.

C) suggest to coccinelle that maybe they include a similar Git based range check option, to avoid needing a fancy script [using Junio's rationale].

D) more clearly identify _why_ the particular instances that are corrected are _worth_ changing now (e.g. fixing _one_ may be a worthy GSOC or Outreachy activity, or .. <insert own reasons>).

 Hope that helps.

Philip

Show 10 quoted lines

>> diff -u -p a/compat/fsmonitor/fsm-listen-darwin.c b/compat/fsmonitor/fsm-listen-darwin.c
>> --- a/compat/fsmonitor/fsm-listen-darwin.c
>> +++ b/compat/fsmonitor/fsm-listen-darwin.c
>> @@ -342,7 +342,7 @@ int fsm_listen__ctor(struct fsmonitor_da
>>                                            data->cfar_paths_to_watch,
>>                                            kFSEventStreamEventIdSinceNow,
>>                                            0.001, flags);
>> -       if (data->stream == NULL)
>> +       if (!data->stream)
>>                 goto failed;

[snipped]