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

From: Junio C Hamano <gitster@pobox.com>
To: Philip Oakley <philipoakley@iee.email>
Cc: Elia Pinto <gitter.spiros@gmail.com>, git@vger.kernel.org
Subject: Re: [PATCH 01/23] contrib/coccinnelle: add equals-null.cocci
Date: Mon, 02 May 2022 09:32:01 -0700	[thread overview]
Message-ID: <xmqqee1b28vy.fsf@gitster.g> (raw)
In-Reply-To: <13dc6ee6-f4ee-c246-b610-ec3d0a72a4ed@iee.email> (Philip Oakley's message of "Mon, 2 May 2022 11:00:48 +0100")

Philip Oakley <philipoakley@iee.email> writes:

>>> As I assumed that applying the patches in this series would create
>>> the branch B, and then I saw that the tip of 'seen' after merging
>>> this topic still needed to have a lot more fixes according to "make
>>> coccicheck", I got a (false) impression that there are too many new
>>> violations from topics in flight, which was the primary source of my
>>> negative reaction against potential code churn.  If we try the above
>>> exercise, perhaps there may not be too many topics that need fix-up
>>> beyond what we fix in the branch B, and if that is the case, I would
>>> not be so negative.
>> So I tried that myself, and the topic branch B was fairly
>> straightforward to create.
>>
>> We have ~60 topics in flight (not counting this one), and it turns
>> out that there is no topic that introduces new code that fails the
>> equals-null.cocci rule.  IOW, the follow-up fixup per topic turns
>> out to be an empty set.
>>
>> So, I'd probably use the [01/23] and then a single ~5k lines patch
>> that was generated with equals-null.cocci rule as the branch B
>> above, let it percolate down from 'seen' to 'next' to eventually
>> 'master'.
>>
>> Thanks.
> That sounds like a good result.
>
> It may also be worth Elia cross checking against a previous release
> (v2.35.0?) for relatively recent introductions, to cover the potential
> revert scenario, just in case..

Sounds sensible.  We do have some changes between 2.35 and 2.36 and
the fork-points of many topics predate 2.36 (and may even 2.35).

Here is an experiment I just did:

 * Applied the patch to add equals-null.cocci to maint-2.35.

 * Ran "coccicheck", applied the resulting equals-null fix and
   committed the result.

 * Merged the "branch B" from last night to it.

The resulting tree exactly matched "branch B" (i.e. 2.36.0 fixed
with equals-null.cocci check).

If I instead merge vanilla 2.36 with the result of fixing
maint-2.35, that differs at two places from "branch B" (i.e. we
added two new violations to 2.36 relative to 2.35).

Doing the same between maint-2.35 and maint-2.34 seems to indicate
that we didn't add any new violations during that period.

So in short, 2.35 may probably be a good place to start, but basing
on 2.36 seems to be good enough.

Thanks.

 branch.c                             | 2 +-
 compat/fsmonitor/fsm-listen-darwin.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git c/branch.c w/branch.c
index bde705b092..d0ca2b76d2 100644
--- c/branch.c
+++ w/branch.c
@@ -653,7 +653,7 @@ void create_branches_recursively(struct repository *r, const char *name,
 	 * be created in every submodule.
 	 */
 	for (i = 0; i < submodule_entry_list.entry_nr; i++) {
-		if (!submodule_entry_list.entries[i].repo) {
+		if (submodule_entry_list.entries[i].repo == NULL) {
 			int code = die_message(
 				_("submodule '%s': unable to find submodule"),
 				submodule_entry_list.entries[i].submodule->name);
diff --git c/compat/fsmonitor/fsm-listen-darwin.c w/compat/fsmonitor/fsm-listen-darwin.c
index dc8a33130a..0741fe834c 100644
--- c/compat/fsmonitor/fsm-listen-darwin.c
+++ w/compat/fsmonitor/fsm-listen-darwin.c
@@ -342,7 +342,7 @@ int fsm_listen__ctor(struct fsmonitor_daemon_state *state)
 					   data->cfar_paths_to_watch,
 					   kFSEventStreamEventIdSinceNow,
 					   0.001, flags);
-	if (!data->stream)
+	if (data->stream == NULL)
 		goto failed;
 
 	/*
