[PATCH 00/23] add a new coccinelle semantic patch to enforce a

[PATCH 00/23] add a new coccinelle semantic patch to enforce a

[Elia Pinto]

This patch series adds a new coccinelle semantic patch to enforce the git coding
style that requires "Do not explicitly compute an integral value with constant 0
or '\ 0', or a pointer value with constant NULL."

It also fixes a set of sources using the new semantic patch.

Elia Pinto (23):
  contrib/coccinnelle: add equals-null.cocci
  apply.c: Fix coding style
  archive.c: Fix coding style
  blame.c: Fix coding style
  branch.c: Fix coding style
  builtin/bisect--helper.c: Fix coding style
  builtin/checkout.c: Fix coding style
  builtin/clone.c: Fix coding style
  builtin/commit.c: Fix coding style
  builtin/diff.c: Fix coding style
  builtin/gc.c: Fix coding style
  builtin/index-pack.c: Fix coding style
  builtin/log.c: Fix coding style
  builtin/ls-remote.c: Fix coding style
  builtin/mailsplit.c: Fix coding style
  builtin/pack-redundant.c: Fix coding style
  builtin/receive-pack.c: Fix coding style
  builtin/replace.c: Fix coding style
  builtin/rev-parse.c: Fix coding style
  builtin/shortlog.c: Fix coding style
  builtin/tag.c: Fix coding style
  combine-diff.c: Fix coding style
  commit-graph.c: Fix coding style

 apply.c                              |  6 +++---
 archive.c                            |  2 +-
 blame.c                              |  6 +++---
 branch.c                             |  4 ++--
 builtin/bisect--helper.c             |  2 +-
 builtin/checkout.c                   |  2 +-
 builtin/clone.c                      |  4 ++--
 builtin/commit.c                     |  2 +-
 builtin/diff.c                       |  2 +-
 builtin/gc.c                         |  6 +++---
 builtin/index-pack.c                 |  6 +++---
 builtin/log.c                        |  2 +-
 builtin/ls-remote.c                  |  2 +-
 builtin/mailsplit.c                  |  2 +-
 builtin/pack-redundant.c             | 10 +++++-----
 builtin/receive-pack.c               |  4 ++--
 builtin/replace.c                    |  2 +-
 builtin/rev-parse.c                  |  2 +-
 builtin/shortlog.c                   |  2 +-
 builtin/tag.c                        |  4 ++--
 combine-diff.c                       |  4 ++--
 commit-graph.c                       |  4 ++--
 contrib/coccinelle/equals-null.cocci | 30 ++++++++++++++++++++++++++++
 23 files changed, 70 insertions(+), 40 deletions(-)
 create mode 100644 contrib/coccinelle/equals-null.cocci

-- 
2.35.1

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

[Elia Pinto]

Add a coccinelle semantic patch necessary to reinforce the git coding style
guideline:

"Do not explicitly compute an integral value with constant 0 or '\ 0', or a
pointer value with constant NULL."

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 contrib/coccinelle/equals-null.cocci | 30 ++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)
 create mode 100644 contrib/coccinelle/equals-null.cocci

diff --git a/contrib/coccinelle/equals-null.cocci b/contrib/coccinelle/equals-null.cocci
new file mode 100644
index 0000000000..92c7054013
--- /dev/null
+++ b/contrib/coccinelle/equals-null.cocci
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+@@
+expression e;
+statement s;
+@@
+if (
+(
+!e
+|
+- e == NULL
++ !e
+)
+   )
+   {...}
+else s
+
+@@
+expression e;
+statement s;
+@@
+if (
+(
+e
+|
+- e != NULL
++ e
+)
+   )
+   {...}
+else s
-- 
2.35.1

[PATCH 02/23] apply.c: Fix coding style

[Elia Pinto]

Adhere to the git coding style which requires "Do not explicitly compute an
integral value with constant 0 or '\ 0', or a pointer value with constant NULL."

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 apply.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/apply.c b/apply.c
index d19c26d332..2b7cd930ef 100644
--- a/apply.c
+++ b/apply.c
@@ -3274,11 +3274,11 @@ static struct patch *in_fn_table(struct apply_state *state, const char *name)
 {
 	struct string_list_item *item;
 
-	if (name == NULL)
+	if (!name)
 		return NULL;
 
 	item = string_list_lookup(&state->fn_table, name);
-	if (item != NULL)
+	if (item)
 		return (struct patch *)item->util;
 
 	return NULL;
@@ -3318,7 +3318,7 @@ static void add_to_fn_table(struct apply_state *state, struct patch *patch)
 	 * This should cover the cases for normal diffs,
 	 * file creations and copies
 	 */
-	if (patch->new_name != NULL) {
+	if (patch->new_name) {
 		item = string_list_insert(&state->fn_table, patch->new_name);
 		item->util = patch;
 	}
-- 
2.35.1

[PATCH 03/23] archive.c: Fix coding style

[Elia Pinto]

Adhere to the git coding style which requires "Do not explicitly compute an
integral value with constant 0 or '\ 0', or a pointer value with constant NULL."

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 archive.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/archive.c b/archive.c
index e29d0e00f6..654ea2a6ed 100644
--- a/archive.c
+++ b/archive.c
@@ -465,7 +465,7 @@ static void parse_treeish_arg(const char **argv,
 	}
 
 	tree = parse_tree_indirect(&oid);
-	if (tree == NULL)
+	if (!tree)
 		die(_("not a tree object: %s"), oid_to_hex(&oid));
 
 	if (prefix) {
-- 
2.35.1

[PATCH 04/23] blame.c: Fix coding style

[Elia Pinto]

Adhere to the git coding style which requires "Do not explicitly compute an
integral value with constant 0 or '\ 0', or a pointer value with constant NULL."

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 blame.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/blame.c b/blame.c
index 186ad96120..da1052ac94 100644
--- a/blame.c
+++ b/blame.c
@@ -1072,7 +1072,7 @@ static struct blame_entry *blame_merge(struct blame_entry *list1,
 	if (p1->s_lno <= p2->s_lno) {
 		do {
 			tail = &p1->next;
-			if ((p1 = *tail) == NULL) {
+			if (!(p1 = *tail)) {
 				*tail = p2;
 				return list1;
 			}
@@ -1082,7 +1082,7 @@ static struct blame_entry *blame_merge(struct blame_entry *list1,
 		*tail = p2;
 		do {
 			tail = &p2->next;
-			if ((p2 = *tail) == NULL)  {
+			if (!(p2 = *tail))  {
 				*tail = p1;
 				return list1;
 			}
@@ -1090,7 +1090,7 @@ static struct blame_entry *blame_merge(struct blame_entry *list1,
 		*tail = p1;
 		do {
 			tail = &p1->next;
-			if ((p1 = *tail) == NULL) {
+			if (!(p1 = *tail)) {
 				*tail = p2;
 				return list1;
 			}
-- 
2.35.1

[PATCH 05/23] branch.c: Fix coding style

[Elia Pinto]

Adhere to the git coding style which requires "Do not explicitly compute an
integral value with constant 0 or '\ 0', or a pointer value with constant NULL."

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 branch.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/branch.c b/branch.c
index 01ecb816d5..bde705b092 100644
--- a/branch.c
+++ b/branch.c
@@ -466,7 +466,7 @@ static void dwim_branch_start(struct repository *r, const char *start_name,
 		break;
 	}
 
-	if ((commit = lookup_commit_reference(r, &oid)) == NULL)
+	if (!(commit = lookup_commit_reference(r, &oid)))
 		die(_("not a valid branch point: '%s'"), start_name);
 	if (out_real_ref) {
 		*out_real_ref = real_ref;
@@ -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 == NULL) {
+		if (!submodule_entry_list.entries[i].repo) {
 			int code = die_message(
 				_("submodule '%s': unable to find submodule"),
 				submodule_entry_list.entries[i].submodule->name);
-- 
2.35.1

[PATCH 06/23] builtin/bisect--helper.c: Fix coding style

[Elia Pinto]

Adhere to the git coding style which requires "Do not explicitly compute an
integral value with constant 0 or '\ 0', or a pointer value with constant NULL."

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 builtin/bisect--helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 8b2b259ff0..e432e1b0ef 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -433,7 +433,7 @@ static int bisect_terms(struct bisect_terms *terms, const char *option)
 	if (get_terms(terms))
 		return error(_("no terms defined"));
 
-	if (option == NULL) {
+	if (!option) {
 		printf(_("Your current terms are %s for the old state\n"
 			 "and %s for the new state.\n"),
 		       terms->term_good, terms->term_bad);
-- 
2.35.1

[PATCH 07/23] builtin/checkout.c: Fix coding style

[Elia Pinto]

Adhere to the git coding style which requires "Do not explicitly compute an
integral value with constant 0 or '\ 0', or a pointer value with constant NULL."

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 builtin/checkout.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 797681481d..f988936ddf 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -834,7 +834,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
 			if (ret)
 				return ret;
 			o.ancestor = old_branch_info->name;
-			if (old_branch_info->name == NULL) {
+			if (!old_branch_info->name) {
 				strbuf_add_unique_abbrev(&old_commit_shortname,
 							 &old_branch_info->commit->object.oid,
 							 DEFAULT_ABBREV);
-- 
2.35.1

[PATCH 08/23] builtin/clone.c: Fix coding style

[Elia Pinto]

Adhere to the git coding style which requires "Do not explicitly compute an
integral value with constant 0 or '\ 0', or a pointer value with constant NULL."

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 builtin/clone.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 5231656379..05b004bc0d 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1106,10 +1106,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	 * apply the remote name provided by --origin only after this second
 	 * call to git_config, to ensure it overrides all config-based values.
 	 */
-	if (option_origin != NULL)
+	if (option_origin)
 		remote_name = xstrdup(option_origin);
 
-	if (remote_name == NULL)
+	if (!remote_name)
 		remote_name = xstrdup("origin");
 
 	if (!valid_remote_name(remote_name))
-- 
2.35.1

[PATCH 09/23] builtin/commit.c: Fix coding style

[Elia Pinto]

Adhere to the git coding style which requires "Do not explicitly compute an
integral value with constant 0 or '\ 0', or a pointer value with constant NULL."

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 builtin/commit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 009a1de0a3..7316fbba1d 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -861,7 +861,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	}
 
 	s->fp = fopen_for_writing(git_path_commit_editmsg());
-	if (s->fp == NULL)
+	if (!s->fp)
 		die_errno(_("could not open '%s'"), git_path_commit_editmsg());
 
 	/* Ignore status.displayCommentPrefix: we do need comments in COMMIT_EDITMSG. */
-- 
2.35.1

[PATCH 10/23] builtin/diff.c: Fix coding style

[Elia Pinto]

Adhere to the git coding style which requires "Do not explicitly compute an
integral value with constant 0 or '\ 0', or a pointer value with constant NULL."

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 builtin/diff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/diff.c b/builtin/diff.c
index bb7fafca61..3397f44d5a 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -352,7 +352,7 @@ static void symdiff_prepare(struct rev_info *rev, struct symdiff *sym)
 			othercount++;
 			continue;
 		}
-		if (map == NULL)
+		if (!map)
 			map = bitmap_new();
 		bitmap_set(map, i);
 	}
-- 
2.35.1

[PATCH 11/23] builtin/gc.c: Fix coding style

[Elia Pinto]

Adhere to the git coding style which requires "Do not explicitly compute an
integral value with constant 0 or '\ 0', or a pointer value with constant NULL."

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 builtin/gc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index b335cffa33..daa4535f1c 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -446,7 +446,7 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid)
 			fscanf(fp, scan_fmt, &pid, locking_host) == 2 &&
 			/* be gentle to concurrent "gc" on remote hosts */
 			(strcmp(locking_host, my_host) || !kill(pid, 0) || errno == EPERM);
-		if (fp != NULL)
+		if (fp)
 			fclose(fp);
 		if (should_exit) {
 			if (fd >= 0)
@@ -2238,7 +2238,7 @@ static int systemd_timer_write_unit_templates(const char *exec_path)
 		goto error;
 	}
 	file = fopen_or_warn(filename, "w");
-	if (file == NULL)
+	if (!file)
 		goto error;
 
 	unit = "# This file was created and is maintained by Git.\n"
@@ -2267,7 +2267,7 @@ static int systemd_timer_write_unit_templates(const char *exec_path)
 
 	filename = xdg_config_home_systemd("git-maintenance@.service");
 	file = fopen_or_warn(filename, "w");
-	if (file == NULL)
+	if (!file)
 		goto error;
 
 	unit = "# This file was created and is maintained by Git.\n"
-- 
2.35.1

[PATCH 12/23] builtin/index-pack.c: Fix coding style

[Elia Pinto]

Adhere to the git coding style which requires "Do not explicitly compute an
integral value with constant 0 or '\ 0', or a pointer value with constant NULL."

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 builtin/index-pack.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 680b66b063..3e385b4800 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1942,11 +1942,11 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
 	free(objects);
 	strbuf_release(&index_name_buf);
 	strbuf_release(&rev_index_name_buf);
-	if (pack_name == NULL)
+	if (!pack_name)
 		free((void *) curr_pack);
-	if (index_name == NULL)
+	if (!index_name)
 		free((void *) curr_index);
-	if (rev_index_name == NULL)
+	if (!rev_index_name)
 		free((void *) curr_rev_index);
 
 	/*
-- 
2.35.1

[PATCH 13/23] builtin/log.c: Fix coding style

[Elia Pinto]

Adhere to the git coding style which requires "Do not explicitly compute an
integral value with constant 0 or '\ 0', or a pointer value with constant NULL."

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 builtin/log.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/log.c b/builtin/log.c
index c211d66d1d..d35419e489 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1012,7 +1012,7 @@ static int open_next_file(struct commit *commit, const char *subject,
 	if (!quiet)
 		printf("%s\n", filename.buf + outdir_offset);
 
-	if ((rev->diffopt.file = fopen(filename.buf, "w")) == NULL) {
+	if (!(rev->diffopt.file = fopen(filename.buf, "w"))) {
 		error_errno(_("cannot open patch file %s"), filename.buf);
 		strbuf_release(&filename);
 		return -1;
-- 
2.35.1

[PATCH 14/23] builtin/ls-remote.c: Fix coding style

[Elia Pinto]

Adhere to the git coding style which requires "Do not explicitly compute an
integral value with constant 0 or '\ 0', or a pointer value with constant NULL."

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 builtin/ls-remote.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index d856085e94..df44e5cc0d 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -114,7 +114,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 	}
 
 	transport = transport_get(remote, NULL);
-	if (uploadpack != NULL)
+	if (uploadpack)
 		transport_set_option(transport, TRANS_OPT_UPLOADPACK, uploadpack);
 	if (server_options.nr)
 		transport->server_options = &server_options;
-- 
2.35.1

[PATCH 15/23] builtin/mailsplit.c: Fix coding style

[Elia Pinto]

Adhere to the git coding style which requires "Do not explicitly compute an
integral value with constant 0 or '\ 0', or a pointer value with constant NULL."

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 builtin/mailsplit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/mailsplit.c b/builtin/mailsplit.c
index 30952353a3..73509f651b 100644
--- a/builtin/mailsplit.c
+++ b/builtin/mailsplit.c
@@ -120,7 +120,7 @@ static int populate_maildir_list(struct string_list *list, const char *path)
 	for (sub = subs; *sub; ++sub) {
 		free(name);
 		name = xstrfmt("%s/%s", path, *sub);
-		if ((dir = opendir(name)) == NULL) {
+		if (!(dir = opendir(name))) {
 			if (errno == ENOENT)
 				continue;
 			error_errno("cannot opendir %s", name);
-- 
2.35.1

[PATCH 16/23] builtin/pack-redundant.c: Fix coding style

[Elia Pinto]

Adhere to the git coding style which requires "Do not explicitly compute an
integral value with constant 0 or '\ 0', or a pointer value with constant NULL."

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 builtin/pack-redundant.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index 8bf5c0acad..ed9b9013a5 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c
@@ -101,7 +101,7 @@ static inline struct llist_item *llist_insert(struct llist *list,
 	oidread(&new_item->oid, oid);
 	new_item->next = NULL;
 
-	if (after != NULL) {
+	if (after) {
 		new_item->next = after->next;
 		after->next = new_item;
 		if (after == list->back)
@@ -157,7 +157,7 @@ static inline struct llist_item * llist_sorted_remove(struct llist *list, const
 		if (cmp > 0) /* not in list, since sorted */
 			return prev;
 		if (!cmp) { /* found */
-			if (prev == NULL) {
+			if (!prev) {
 				if (hint != NULL && hint != list->front) {
 					/* we don't know the previous element */
 					hint = NULL;
@@ -219,7 +219,7 @@ static struct pack_list * pack_list_difference(const struct pack_list *A,
 	struct pack_list *ret;
 	const struct pack_list *pl;
 
-	if (A == NULL)
+	if (!A)
 		return NULL;
 
 	pl = B;
@@ -317,7 +317,7 @@ static size_t get_pack_redundancy(struct pack_list *pl)
 	struct pack_list *subset;
 	size_t ret = 0;
 
-	if (pl == NULL)
+	if (!pl)
 		return 0;
 
 	while ((subset = pl->next)) {
@@ -611,7 +611,7 @@ int cmd_pack_redundant(int argc, const char **argv, const char *prefix)
 		while (*(argv + i) != NULL)
 			add_pack_file(*(argv + i++));
 
-	if (local_packs == NULL)
+	if (!local_packs)
 		die("Zero packs found!");
 
 	load_all_objects();
-- 
2.35.1

[PATCH 17/23] builtin/receive-pack.c: Fix coding style

[Elia Pinto]

Adhere to the git coding style which requires "Do not explicitly compute an
integral value with constant 0 or '\ 0', or a pointer value with constant NULL."

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 builtin/receive-pack.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 9aabffa1af..ad20b41e3c 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1664,7 +1664,7 @@ static void check_aliased_update_internal(struct command *cmd,
 	}
 	dst_name = strip_namespace(dst_name);
 
-	if ((item = string_list_lookup(list, dst_name)) == NULL)
+	if (!(item = string_list_lookup(list, dst_name)))
 		return;
 
 	cmd->skip_update = 1;
@@ -2538,7 +2538,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 			   PACKET_READ_CHOMP_NEWLINE |
 			   PACKET_READ_DIE_ON_ERR_PACKET);
 
-	if ((commands = read_head_info(&reader, &shallow)) != NULL) {
+	if ((commands = read_head_info(&reader, &shallow))) {
 		const char *unpack_status = NULL;
 		struct string_list push_options = STRING_LIST_INIT_DUP;
 
-- 
2.35.1

[PATCH 18/23] builtin/replace.c: Fix coding style

[Elia Pinto]

Adhere to the git coding style which requires "Do not explicitly compute an
integral value with constant 0 or '\ 0', or a pointer value with constant NULL."

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 builtin/replace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/replace.c b/builtin/replace.c
index 5068f4f0b2..583702a098 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -72,7 +72,7 @@ static int list_replace_refs(const char *pattern, const char *format)
 {
 	struct show_data data;
 
-	if (pattern == NULL)
+	if (!pattern)
 		pattern = "*";
 	data.pattern = pattern;
 
-- 
2.35.1

[PATCH 19/23] builtin/rev-parse.c: Fix coding style

[Elia Pinto]

Adhere to the git coding style which requires "Do not explicitly compute an
integral value with constant 0 or '\ 0', or a pointer value with constant NULL."

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 builtin/rev-parse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 8480a59f57..660a6a749b 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -476,7 +476,7 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
 
 		/* name(s) */
 		s = strpbrk(sb.buf, flag_chars);
-		if (s == NULL)
+		if (!s)
 			s = help;
 
 		if (s - sb.buf == 1) /* short option only */
-- 
2.35.1

[PATCH 20/23] builtin/shortlog.c: Fix coding style

[Elia Pinto]

Adhere to the git coding style which requires "Do not explicitly compute an
integral value with constant 0 or '\ 0', or a pointer value with constant NULL."

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 builtin/shortlog.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 26c5c0cf93..62c4a4eaba 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -81,7 +81,7 @@ static void insert_one_record(struct shortlog *log,
 		format_subject(&subject, oneline, " ");
 		buffer = strbuf_detach(&subject, NULL);
 
-		if (item->util == NULL)
+		if (!item->util)
 			item->util = xcalloc(1, sizeof(struct string_list));
 		string_list_append(item->util, buffer);
 	}
-- 
2.35.1

[PATCH 21/23] builtin/tag.c: Fix coding style

[Elia Pinto]

Adhere to the git coding style which requires "Do not explicitly compute an
integral value with constant 0 or '\ 0', or a pointer value with constant NULL."

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 builtin/tag.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index e5a8f85693..75dece0e4f 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -364,7 +364,7 @@ static void create_reflog_msg(const struct object_id *oid, struct strbuf *sb)
 		strbuf_addstr(sb, "object of unknown type");
 		break;
 	case OBJ_COMMIT:
-		if ((buf = read_object_file(oid, &type, &size)) != NULL) {
+		if ((buf = read_object_file(oid, &type, &size))) {
 			subject_len = find_commit_subject(buf, &subject_start);
 			strbuf_insert(sb, sb->len, subject_start, subject_len);
 		} else {
@@ -372,7 +372,7 @@ static void create_reflog_msg(const struct object_id *oid, struct strbuf *sb)
 		}
 		free(buf);
 
-		if ((c = lookup_commit_reference(the_repository, oid)) != NULL)
+		if ((c = lookup_commit_reference(the_repository, oid)))
 			strbuf_addf(sb, ", %s", show_date(c->date, 0, DATE_MODE(SHORT)));
 		break;
 	case OBJ_TREE:
-- 
2.35.1

[PATCH 22/23] combine-diff.c: Fix coding style

[Elia Pinto]

Adhere to the git coding style which requires "Do not explicitly compute an
integral value with constant 0 or '\ 0', or a pointer value with constant NULL."

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 combine-diff.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/combine-diff.c b/combine-diff.c
index d93782daeb..b724f02123 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -195,10 +195,10 @@ static struct lline *coalesce_lines(struct lline *base, int *lenbase,
 	struct lline *baseend, *newend = NULL;
 	int i, j, origbaselen = *lenbase;
 
-	if (newline == NULL)
+	if (!newline)
 		return base;
 
-	if (base == NULL) {
+	if (!base) {
 		*lenbase = lennew;
 		return newline;
 	}
-- 
2.35.1

[PATCH 23/23] commit-graph.c: Fix coding style

[Elia Pinto]

Adhere to the git coding style which requires "Do not explicitly compute an
integral value with constant 0 or '\ 0', or a pointer value with constant NULL."

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 commit-graph.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/commit-graph.c b/commit-graph.c
index 441b36016b..fcd351ee00 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -2567,7 +2567,7 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g, int flags)
 		odb_parents = odb_commit->parents;
 
 		while (graph_parents) {
-			if (odb_parents == NULL) {
+			if (!odb_parents) {
 				graph_report(_("commit-graph parent list for commit %s is too long"),
 					     oid_to_hex(&cur_oid));
 				break;
@@ -2590,7 +2590,7 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g, int flags)
 			odb_parents = odb_parents->next;
 		}
 
-		if (odb_parents != NULL)
+		if (odb_parents)
 			graph_report(_("commit-graph parent list for commit %s terminates early"),
 				     oid_to_hex(&cur_oid));
 
-- 
2.35.1

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

[Philip Oakley]

On 30/04/2022 05:13, Elia Pinto wrote:
> Add a coccinelle semantic patch necessary to reinforce the git coding style
> guideline:
>
> "Do not explicitly compute an integral value with constant 0 or '\ 0', or a

s/compute/compare/
> pointer value with constant NULL."

If this gets re-rolled, perhaps include a simple example for those who
don't immediately understand that quoted sentence. It will also help
decode the coccinelle script

so:     `if (ptr == NULL)` becomes `if (!ptr)`  etc.

--
Philip
>
> Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
> ---
>  contrib/coccinelle/equals-null.cocci | 30 ++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>  create mode 100644 contrib/coccinelle/equals-null.cocci
>
> diff --git a/contrib/coccinelle/equals-null.cocci b/contrib/coccinelle/equals-null.cocci
> new file mode 100644
> index 0000000000..92c7054013
> --- /dev/null
> +++ b/contrib/coccinelle/equals-null.cocci
> @@ -0,0 +1,30 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +@@
> +expression e;
> +statement s;
> +@@
> +if (
> +(
> +!e
> +|
> +- e == NULL
> ++ !e
> +)
> +   )
> +   {...}
> +else s
> +
> +@@
> +expression e;
> +statement s;
> +@@
> +if (
> +(
> +e
> +|
> +- e != NULL
> ++ e
> +)
> +   )
> +   {...}
> +else s

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

[Junio C Hamano]

Philip Oakley <philipoakley@iee.email> writes:

> On 30/04/2022 05:13, Elia Pinto wrote:
>> Add a coccinelle semantic patch necessary to reinforce the git coding style
>> guideline:
>>
>> "Do not explicitly compute an integral value with constant 0 or '\ 0', or a
>
> s/compute/compare/
>> pointer value with constant NULL."
>
> If this gets re-rolled, perhaps include a simple example for those who
> don't immediately understand that quoted sentence. It will also help
> decode the coccinelle script
>
> so:     `if (ptr == NULL)` becomes `if (!ptr)`  etc.

That is certainly a good suggestion, but I am wondering if we want
to also emphasize another more generally applicable rule that
appears much earlier in the guideline document:

 - Fixing style violations while working on a real change as a
   preparatory clean-up step is good, but otherwise avoid useless code
   churn for the sake of conforming to the style.

   "Once it _is_ in the tree, it's not really worth the patch noise to
   go and fix it up."
   Cf. http://lkml.iu.edu/hypermail/linux/kernel/1001.3/01069.html

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

[Philip Oakley]

On 30/04/2022 21:56, Junio C Hamano wrote:
> Philip Oakley <philipoakley@iee.email> writes:
>
>> On 30/04/2022 05:13, Elia Pinto wrote:
>>> Add a coccinelle semantic patch necessary to reinforce the git coding style
>>> guideline:
>>>
>>> "Do not explicitly compute an integral value with constant 0 or '\ 0', or a
>> s/compute/compare/
>>> pointer value with constant NULL."
>> If this gets re-rolled, perhaps include a simple example for those who
>> don't immediately understand that quoted sentence. It will also help
>> decode the coccinelle script
>>
>> so:     `if (ptr == NULL)` becomes `if (!ptr)`  etc.
> That is certainly a good suggestion, but I am wondering if we want
> to also emphasize another more generally applicable rule that
> appears much earlier in the guideline document:
>
>  - Fixing style violations while working on a real change as a
>    preparatory clean-up step is good, but otherwise avoid useless code
>    churn for the sake of conforming to the style.
>
>    "Once it _is_ in the tree, it's not really worth the patch noise to
>    go and fix it up."
>    Cf. http://lkml.iu.edu/hypermail/linux/kernel/1001.3/01069.html

I think it goes both ways when the 'bad' style can be cargo-cult copied
too easily, negating the value of the guidance.

That said, having 22 patches to renormalise the codebase does end up as
being excessive. And it's not clear if the first cocci patch ends up as
part of the regular lint checking (I'm not a user of cocci..).

I suspect that all the renormalising fixes were the result of a single
cocci check, so having a single patch that makes the codebase clean
would be better, if accepted.
--
Philip

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

[Junio C Hamano]

Philip Oakley <philipoakley@iee.email> writes:

> I think it goes both ways when the 'bad' style can be cargo-cult copied
> too easily, negating the value of the guidance.

Yes, and the cocci rules by themselves do not help protecting our
codebase against it all that much.

In order to help developers follow the guideline to avoid adding
_new_ instances (by copying-and-pasting), it should be easy to use
such a checker in such a way that we can notice only _new_ breakges
while ignoring existing offenders.  I do not think the current cocci
check target in our Makefile is prepared for that.

And there are two ways to deal with that shortcoming.

One, which often appears easier to implement but in the medium term
is very costly, is to freeze the codebase and apply tree-wide code
churn to make warning disappear.

Then _any_ breakages noticed by an inadequate tool, which does not
allow us to notice only the new breakages, after applying a patch to
such a cleansed codebase by definition are coming from the patch.

But it is costly.  The codebase is rarely frozen, so there isn't a
good time to apply such a patch, whether it is 22-patch series or a
single patch that concatenates everything into one.  There may be
more urgent issues than style fixes that would force us to revert a
change made before such a tree-wide clean-up, and when that happens,
such a "clean-up for clean-up's sake because we cannot check
incrementally" will inevitably conflict with such a change.

The other approach is to make it possible (and easy) to check
incrementally, so that we can detect new instances made by copying
and pasting.

Thanks.

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

[Junio C Hamano]

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.



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;
 
 	/*
diff -u -p a/compat/mingw.c b/compat/mingw.c
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1060,7 +1060,7 @@ char *mingw_mktemp(char *template)
 int mkstemp(char *template)
 {
 	char *filename = mktemp(template);
-	if (filename == NULL)
+	if (!filename)
 		return -1;
 	return open(filename, O_RDWR | O_CREAT, 0600);
 }
@@ -2332,7 +2332,7 @@ int setitimer(int type, struct itimerval
 	static const struct timeval zero;
 	static int atexit_done;
 
-	if (out != NULL)
+	if (out)
 		return errno = EINVAL,
 			error("setitimer param 3 != NULL not implemented");
 	if (!is_timeval_eq(&in->it_interval, &zero) &&
@@ -2361,7 +2361,7 @@ int sigaction(int sig, struct sigaction
 	if (sig != SIGALRM)
 		return errno = EINVAL,
 			error("sigaction only implemented for SIGALRM");
-	if (out != NULL)
+	if (out)
 		return errno = EINVAL,
 			error("sigaction: param 3 != NULL not implemented");
 
diff -u -p a/compat/mkdir.c b/compat/mkdir.c
--- a/compat/mkdir.c
+++ b/compat/mkdir.c
@@ -9,7 +9,7 @@ int compat_mkdir_wo_trailing_slash(const
 	size_t len = strlen(dir);
 
 	if (len && dir[len-1] == '/') {
-		if ((tmp_dir = strdup(dir)) == NULL)
+		if (!(tmp_dir = strdup(dir)))
 			return -1;
 		tmp_dir[len-1] = '\0';
 	}
diff -u -p a/compat/mmap.c b/compat/mmap.c
--- a/compat/mmap.c
+++ b/compat/mmap.c
@@ -13,7 +13,7 @@ void *git_mmap(void *start, size_t lengt
 	}
 
 	start = malloc(length);
-	if (start == NULL) {
+	if (!start) {
 		errno = ENOMEM;
 		return MAP_FAILED;
 	}
diff -u -p a/compat/mingw.c b/compat/mingw.c
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1060,7 +1060,7 @@ char *mingw_mktemp(char *template)
 int mkstemp(char *template)
 {
 	char *filename = mktemp(template);
-	if (filename == NULL)
+	if (!filename)
 		return -1;
 	return open(filename, O_RDWR | O_CREAT, 0600);
 }
@@ -2332,7 +2332,7 @@ int setitimer(int type, struct itimerval
 	static const struct timeval zero;
 	static int atexit_done;
 
-	if (out != NULL)
+	if (out)
 		return errno = EINVAL,
 			error("setitimer param 3 != NULL not implemented");
 	if (!is_timeval_eq(&in->it_interval, &zero) &&
@@ -2361,7 +2361,7 @@ int sigaction(int sig, struct sigaction
 	if (sig != SIGALRM)
 		return errno = EINVAL,
 			error("sigaction only implemented for SIGALRM");
-	if (out != NULL)
+	if (out)
 		return errno = EINVAL,
 			error("sigaction: param 3 != NULL not implemented");
 
diff -u -p a/config.c b/config.c
--- a/config.c
+++ b/config.c
@@ -3190,7 +3190,7 @@ int git_config_set_multivar_in_file_gent
 			goto out_free;
 		}
 		/* if nothing to unset, error out */
-		if (value == NULL) {
+		if (!value) {
 			ret = CONFIG_NOTHING_SET;
 			goto out_free;
 		}
@@ -3206,7 +3206,7 @@ int git_config_set_multivar_in_file_gent
 		int i, new_line = 0;
 		struct config_options opts;
 
-		if (value_pattern == NULL)
+		if (!value_pattern)
 			store.value_pattern = NULL;
 		else if (value_pattern == CONFIG_REGEX_NONE)
 			store.value_pattern = CONFIG_REGEX_NONE;
@@ -3346,7 +3346,7 @@ int git_config_set_multivar_in_file_gent
 		}
 
 		/* write the pair (value == NULL means unset) */
-		if (value != NULL) {
+		if (value) {
 			if (!store.section_seen) {
 				if (write_section(fd, key, &store) < 0)
 					goto write_err_out;
@@ -3567,7 +3567,7 @@ static int git_config_copy_or_rename_sec
 			offset = section_name_match(&buf[i], old_name);
 			if (offset > 0) {
 				ret++;
-				if (new_name == NULL) {
+				if (!new_name) {
 					remove = 1;
 					continue;
 				}
diff -u -p a/daemon.c b/daemon.c
--- a/daemon.c
+++ b/daemon.c
@@ -447,7 +447,7 @@ static void copy_to_log(int fd)
 	FILE *fp;
 
 	fp = fdopen(fd, "r");
-	if (fp == NULL) {
+	if (!fp) {
 		logerror("fdopen of error channel failed");
 		close(fd);
 		return;
diff -u -p a/dir.c b/dir.c
--- a/dir.c
+++ b/dir.c
@@ -3054,7 +3054,7 @@ char *git_url_basename(const char *repo,
 	 * Skip scheme.
 	 */
 	start = strstr(repo, "://");
-	if (start == NULL)
+	if (!start)
 		start = repo;
 	else
 		start += 3;
diff -u -p a/ewah/bitmap.c b/ewah/bitmap.c
--- a/ewah/bitmap.c
+++ b/ewah/bitmap.c
@@ -223,7 +223,7 @@ void bitmap_reset(struct bitmap *bitmap)
 
 void bitmap_free(struct bitmap *bitmap)
 {
-	if (bitmap == NULL)
+	if (!bitmap)
 		return;
 
 	free(bitmap->words);
diff -u -p a/ewah/ewah_bitmap.c b/ewah/ewah_bitmap.c
--- a/ewah/ewah_bitmap.c
+++ b/ewah/ewah_bitmap.c
@@ -451,7 +451,7 @@ struct ewah_bitmap *ewah_pool_new(void)
 
 void ewah_pool_free(struct ewah_bitmap *self)
 {
-	if (self == NULL)
+	if (!self)
 		return;
 
 	if (bitmap_pool_size == BITMAP_POOL_MAX ||
diff -u -p a/http-fetch.c b/http-fetch.c
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -55,7 +55,7 @@ static void fetch_single_packfile(struct
 	http_init(NULL, url, 0);
 
 	preq = new_direct_http_pack_request(packfile_hash->hash, xstrdup(url));
-	if (preq == NULL)
+	if (!preq)
 		die("couldn't create http pack request");
 	preq->slot->results = &results;
 	preq->index_pack_args = index_pack_args;
diff -u -p a/http-push.c b/http-push.c
--- a/http-push.c
+++ b/http-push.c
@@ -253,7 +253,7 @@ static void start_fetch_loose(struct tra
 	struct http_object_request *obj_req;
 
 	obj_req = new_http_object_request(repo->url, &request->obj->oid);
-	if (obj_req == NULL) {
+	if (!obj_req) {
 		request->state = ABORTED;
 		return;
 	}
@@ -318,7 +318,7 @@ static void start_fetch_packed(struct tr
 	fprintf(stderr, " which contains %s\n", oid_to_hex(&request->obj->oid));
 
 	preq = new_http_pack_request(target->hash, repo->url);
-	if (preq == NULL) {
+	if (!preq) {
 		repo->can_update_info_refs = 0;
 		return;
 	}
@@ -520,7 +520,7 @@ static void finish_request(struct transf
 	/* Keep locks active */
 	check_locks();
 
-	if (request->headers != NULL)
+	if (request->headers)
 		curl_slist_free_all(request->headers);
 
 	/* URL is reused for MOVE after PUT and used during FETCH */
@@ -783,7 +783,7 @@ xml_start_tag(void *userData, const char
 	const char *c = strchr(name, ':');
 	int old_namelen, new_len;
 
-	if (c == NULL)
+	if (!c)
 		c = name;
 	else
 		c++;
@@ -811,7 +811,7 @@ xml_end_tag(void *userData, const char *
 
 	ctx->userFunc(ctx, 1);
 
-	if (c == NULL)
+	if (!c)
 		c = name;
 	else
 		c++;
@@ -1893,7 +1893,7 @@ int cmd_main(int argc, const char **argv
 
 		/* Lock remote branch ref */
 		ref_lock = lock_remote(ref->name, LOCK_TIME);
-		if (ref_lock == NULL) {
+		if (!ref_lock) {
 			fprintf(stderr, "Unable to lock remote branch %s\n",
 				ref->name);
 			if (helper_status)
diff -u -p a/http-walker.c b/http-walker.c
--- a/http-walker.c
+++ b/http-walker.c
@@ -59,7 +59,7 @@ static void start_object_request(struct
 	struct http_object_request *req;
 
 	req = new_http_object_request(obj_req->repo->base, &obj_req->oid);
-	if (req == NULL) {
+	if (!req) {
 		obj_req->state = ABORTED;
 		return;
 	}
@@ -106,7 +106,7 @@ static void process_object_response(void
 	/* Use alternates if necessary */
 	if (missing_target(obj_req->req)) {
 		fetch_alternates(walker, alt->base);
-		if (obj_req->repo->next != NULL) {
+		if (obj_req->repo->next) {
 			obj_req->repo =
 				obj_req->repo->next;
 			release_http_object_request(obj_req->req);
@@ -225,12 +225,12 @@ static void process_alternates_response(
 					 alt_req->url->buf);
 			active_requests++;
 			slot->in_use = 1;
-			if (slot->finished != NULL)
+			if (slot->finished)
 				(*slot->finished) = 0;
 			if (!start_active_slot(slot)) {
 				cdata->got_alternates = -1;
 				slot->in_use = 0;
-				if (slot->finished != NULL)
+				if (slot->finished)
 					(*slot->finished) = 1;
 			}
 			return;
@@ -443,7 +443,7 @@ static int http_fetch_pack(struct walker
 	}
 
 	preq = new_http_pack_request(target->hash, repo->base);
-	if (preq == NULL)
+	if (!preq)
 		goto abort;
 	preq->slot->results = &results;
 
@@ -489,11 +489,11 @@ static int fetch_object(struct walker *w
 		if (hasheq(obj_req->oid.hash, hash))
 			break;
 	}
-	if (obj_req == NULL)
+	if (!obj_req)
 		return error("Couldn't find request for %s in the queue", hex);
 
 	if (has_object_file(&obj_req->oid)) {
-		if (obj_req->req != NULL)
+		if (obj_req->req)
 			abort_http_object_request(obj_req->req);
 		abort_object_request(obj_req);
 		return 0;
diff -u -p a/http.c b/http.c
--- a/http.c
+++ b/http.c
@@ -197,11 +197,11 @@ static void finish_active_slot(struct ac
 	closedown_active_slot(slot);
 	curl_easy_getinfo(slot->curl, CURLINFO_HTTP_CODE, &slot->http_code);
 
-	if (slot->finished != NULL)
+	if (slot->finished)
 		(*slot->finished) = 1;
 
 	/* Store slot results so they can be read after the slot is reused */
-	if (slot->results != NULL) {
+	if (slot->results) {
 		slot->results->curl_result = slot->curl_result;
 		slot->results->http_code = slot->http_code;
 		curl_easy_getinfo(slot->curl, CURLINFO_HTTPAUTH_AVAIL,
@@ -212,7 +212,7 @@ static void finish_active_slot(struct ac
 	}
 
 	/* Run callback if appropriate */
-	if (slot->callback_func != NULL)
+	if (slot->callback_func)
 		slot->callback_func(slot->callback_data);
 }
 
@@ -234,7 +234,7 @@ static void process_curl_messages(void)
 			while (slot != NULL &&
 			       slot->curl != curl_message->easy_handle)
 				slot = slot->next;
-			if (slot != NULL) {
+			if (slot) {
 				xmulti_remove_handle(slot);
 				slot->curl_result = curl_result;
 				finish_active_slot(slot);
@@ -838,16 +838,16 @@ static CURL *get_curl_handle(void)
 		curl_easy_setopt(result, CURLOPT_SSL_CIPHER_LIST,
 				ssl_cipherlist);
 
-	if (ssl_cert != NULL)
+	if (ssl_cert)
 		curl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);
 	if (has_cert_password())
 		curl_easy_setopt(result, CURLOPT_KEYPASSWD, cert_auth.password);
-	if (ssl_key != NULL)
+	if (ssl_key)
 		curl_easy_setopt(result, CURLOPT_SSLKEY, ssl_key);
-	if (ssl_capath != NULL)
+	if (ssl_capath)
 		curl_easy_setopt(result, CURLOPT_CAPATH, ssl_capath);
 #ifdef GIT_CURL_HAVE_CURLOPT_PINNEDPUBLICKEY
-	if (ssl_pinnedkey != NULL)
+	if (ssl_pinnedkey)
 		curl_easy_setopt(result, CURLOPT_PINNEDPUBLICKEY, ssl_pinnedkey);
 #endif
 	if (http_ssl_backend && !strcmp("schannel", http_ssl_backend) &&
@@ -857,10 +857,10 @@ static CURL *get_curl_handle(void)
 		curl_easy_setopt(result, CURLOPT_PROXY_CAINFO, NULL);
 #endif
 	} else if (ssl_cainfo != NULL || http_proxy_ssl_ca_info != NULL) {
-		if (ssl_cainfo != NULL)
+		if (ssl_cainfo)
 			curl_easy_setopt(result, CURLOPT_CAINFO, ssl_cainfo);
 #ifdef GIT_CURL_HAVE_CURLOPT_PROXY_CAINFO
-		if (http_proxy_ssl_ca_info != NULL)
+		if (http_proxy_ssl_ca_info)
 			curl_easy_setopt(result, CURLOPT_PROXY_CAINFO, http_proxy_ssl_ca_info);
 #endif
 	}
@@ -1050,7 +1050,7 @@ void http_init(struct remote *remote, co
 
 	{
 		char *http_max_requests = getenv("GIT_HTTP_MAX_REQUESTS");
-		if (http_max_requests != NULL)
+		if (http_max_requests)
 			max_requests = atoi(http_max_requests);
 	}
 
@@ -1069,10 +1069,10 @@ void http_init(struct remote *remote, co
 	set_from_env(&user_agent, "GIT_HTTP_USER_AGENT");
 
 	low_speed_limit = getenv("GIT_HTTP_LOW_SPEED_LIMIT");
-	if (low_speed_limit != NULL)
+	if (low_speed_limit)
 		curl_low_speed_limit = strtol(low_speed_limit, NULL, 10);
 	low_speed_time = getenv("GIT_HTTP_LOW_SPEED_TIME");
-	if (low_speed_time != NULL)
+	if (low_speed_time)
 		curl_low_speed_time = strtol(low_speed_time, NULL, 10);
 
 	if (curl_ssl_verify == -1)
@@ -1109,7 +1109,7 @@ void http_cleanup(void)
 
 	while (slot != NULL) {
 		struct active_request_slot *next = slot->next;
-		if (slot->curl != NULL) {
+		if (slot->curl) {
 			xmulti_remove_handle(slot);
 			curl_easy_cleanup(slot->curl);
 		}
@@ -1147,13 +1147,13 @@ void http_cleanup(void)
 	free((void *)http_proxy_authmethod);
 	http_proxy_authmethod = NULL;
 
-	if (cert_auth.password != NULL) {
+	if (cert_auth.password) {
 		memset(cert_auth.password, 0, strlen(cert_auth.password));
 		FREE_AND_NULL(cert_auth.password);
 	}
 	ssl_cert_password_required = 0;
 
-	if (proxy_cert_auth.password != NULL) {
+	if (proxy_cert_auth.password) {
 		memset(proxy_cert_auth.password, 0, strlen(proxy_cert_auth.password));
 		FREE_AND_NULL(proxy_cert_auth.password);
 	}
@@ -1179,14 +1179,14 @@ struct active_request_slot *get_active_s
 	while (slot != NULL && slot->in_use)
 		slot = slot->next;
 
-	if (slot == NULL) {
+	if (!slot) {
 		newslot = xmalloc(sizeof(*newslot));
 		newslot->curl = NULL;
 		newslot->in_use = 0;
 		newslot->next = NULL;
 
 		slot = active_queue_head;
-		if (slot == NULL) {
+		if (!slot) {
 			active_queue_head = newslot;
 		} else {
 			while (slot->next != NULL)
@@ -1196,7 +1196,7 @@ struct active_request_slot *get_active_s
 		slot = newslot;
 	}
 
-	if (slot->curl == NULL) {
+	if (!slot->curl) {
 		slot->curl = curl_easy_duphandle(curl_default);
 		curl_session_count++;
 	}
@@ -1768,7 +1768,7 @@ static int http_request(const char *url,
 	slot = get_active_slot();
 	curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);
 
-	if (result == NULL) {
+	if (!result) {
 		curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1);
 	} else {
 		curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
@@ -2100,7 +2100,7 @@ cleanup:
 
 void release_http_pack_request(struct http_pack_request *preq)
 {
-	if (preq->packfile != NULL) {
+	if (preq->packfile) {
 		fclose(preq->packfile);
 		preq->packfile = NULL;
 	}
@@ -2391,7 +2391,7 @@ abort:
 
 void process_http_object_request(struct http_object_request *freq)
 {
-	if (freq->slot == NULL)
+	if (!freq->slot)
 		return;
 	freq->curl_result = freq->slot->curl_result;
 	freq->http_code = freq->slot->http_code;
@@ -2448,7 +2448,7 @@ void release_http_object_request(struct
 		freq->localfile = -1;
 	}
 	FREE_AND_NULL(freq->url);
-	if (freq->slot != NULL) {
+	if (freq->slot) {
 		freq->slot->callback_func = NULL;
 		freq->slot->callback_data = NULL;
 		release_active_slot(freq->slot);
diff -u -p a/kwset.c b/kwset.c
--- a/kwset.c
+++ b/kwset.c
@@ -477,7 +477,7 @@ kwsprep (kwset_t kws)
 	next[i] = NULL;
       treenext(kwset->trie->links, next);
 
-      if ((trans = kwset->trans) != NULL)
+      if ((trans = kwset->trans))
 	for (i = 0; i < NCHAR; ++i)
 	  kwset->next[i] = next[U(trans[i])];
       else
@@ -485,7 +485,7 @@ kwsprep (kwset_t kws)
     }
 
   /* Fix things up for any translation table. */
-  if ((trans = kwset->trans) != NULL)
+  if ((trans = kwset->trans))
     for (i = 0; i < NCHAR; ++i)
       kwset->delta[i] = delta[U(trans[i])];
   else
diff -u -p a/ll-merge.c b/ll-merge.c
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -207,7 +207,7 @@ static enum ll_merge_result ll_ext_merge
 	dict[4].placeholder = "P"; dict[4].value = path_sq.buf;
 	dict[5].placeholder = NULL; dict[5].value = NULL;
 
-	if (fn->cmdline == NULL)
+	if (!fn->cmdline)
 		die("custom merge driver %s lacks command line.", fn->name);
 
 	result->ptr = NULL;
diff -u -p a/log-tree.c b/log-tree.c
--- a/log-tree.c
+++ b/log-tree.c
@@ -88,7 +88,7 @@ static int match_ref_pattern(const char
 			     const struct string_list_item *item)
 {
 	int matched = 0;
-	if (item->util == NULL) {
+	if (!item->util) {
 		if (!wildmatch(item->string, refname, 0))
 			matched = 1;
 	} else {
diff -u -p a/mailinfo.c b/mailinfo.c
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -698,7 +698,7 @@ static int is_scissors_line(const char *
 			continue;
 		}
 		last_nonblank = c;
-		if (first_nonblank == NULL)
+		if (!first_nonblank)
 			first_nonblank = c;
 		if (*c == '-') {
 			in_perforation = 1;
@@ -1094,7 +1094,7 @@ static void handle_body(struct mailinfo
 			 */
 			lines = strbuf_split(line, '\n');
 			for (it = lines; (sb = *it); it++) {
-				if (*(it + 1) == NULL) /* The last line */
+				if (!*(it + 1)) /* The last line */
 					if (sb->buf[sb->len - 1] != '\n') {
 						/* Partial line, save it for later. */
 						strbuf_addbuf(&prev, sb);
diff -u -p a/mailmap.c b/mailmap.c
--- a/mailmap.c
+++ b/mailmap.c
@@ -77,7 +77,7 @@ static void add_mapping(struct string_li
 	struct mailmap_entry *me;
 	struct string_list_item *item;
 
-	if (old_email == NULL) {
+	if (!old_email) {
 		old_email = new_email;
 		new_email = NULL;
 	}
@@ -92,7 +92,7 @@ static void add_mapping(struct string_li
 		item->util = me;
 	}
 
-	if (old_name == NULL) {
+	if (!old_name) {
 		debug_mm("mailmap: adding (simple) entry for '%s'\n", old_email);
 
 		/* Replace current name and new email for simple entry */
@@ -123,9 +123,9 @@ static char *parse_name_and_email(char *
 	char *left, *right, *nstart, *nend;
 	*name = *email = NULL;
 
-	if ((left = strchr(buffer, '<')) == NULL)
+	if (!(left = strchr(buffer, '<')))
 		return NULL;
-	if ((right = strchr(left+1, '>')) == NULL)
+	if (!(right = strchr(left + 1, '>')))
 		return NULL;
 	if (!allow_empty_email && (left+1 == right))
 		return NULL;
@@ -153,7 +153,7 @@ static void read_mailmap_line(struct str
 	if (buffer[0] == '#')
 		return;
 
-	if ((name2 = parse_name_and_email(buffer, &name1, &email1, 0)) != NULL)
+	if ((name2 = parse_name_and_email(buffer, &name1, &email1, 0)))
 		parse_name_and_email(name2, &name2, &email2, 1);
 
 	if (email1)
@@ -320,7 +320,7 @@ int map_user(struct string_list *map,
 		 (int)*emaillen, debug_str(*email));
 
 	item = lookup_prefix(map, *email, *emaillen);
-	if (item != NULL) {
+	if (item) {
 		me = (struct mailmap_entry *)item->util;
 		if (me->namemap.nr) {
 			/*
@@ -334,7 +334,7 @@ int map_user(struct string_list *map,
 				item = subitem;
 		}
 	}
-	if (item != NULL) {
+	if (item) {
 		struct mailmap_info *mi = (struct mailmap_info *)item->util;
 		if (mi->name == NULL && mi->email == NULL) {
 			debug_mm("map_user:  -- (no simple mapping)\n");
diff -u -p a/merge-ort.c b/merge-ort.c
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -2068,7 +2068,7 @@ static char *handle_path_level_conflicts
 	 * to ensure that's the case.
 	 */
 	c_info = strmap_get(collisions, new_path);
-	if (c_info == NULL)
+	if (!c_info)
 		BUG("c_info is NULL");
 
 	/*
@@ -4640,7 +4640,7 @@ static void merge_ort_internal(struct me
 	}
 
 	merged_merge_bases = pop_commit(&merge_bases);
-	if (merged_merge_bases == NULL) {
+	if (!merged_merge_bases) {
 		/* if there is no common ancestor, use an empty tree */
 		struct tree *tree;
 
diff -u -p a/merge-recursive.c b/merge-recursive.c
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -82,7 +82,7 @@ static struct dir_rename_entry *dir_rena
 {
 	struct dir_rename_entry key;
 
-	if (dir == NULL)
+	if (!dir)
 		return NULL;
 	hashmap_entry_init(&key.ent, strhash(dir));
 	key.dir = dir;
@@ -1990,14 +1990,14 @@ static void get_renamed_dir_portion(cons
 	 * renamed means the root directory can never be renamed -- because
 	 * the root directory always exists).
 	 */
-	if (end_of_old == NULL)
+	if (!end_of_old)
 		return; /* Note: *old_dir and *new_dir are still NULL */
 
 	/*
 	 * If new_path contains no directory (end_of_new is NULL), then we
 	 * have a rename of old_path's directory to the root directory.
 	 */
-	if (end_of_new == NULL) {
+	if (!end_of_new) {
 		*old_dir = xstrndup(old_path, end_of_old - old_path);
 		*new_dir = xstrdup("");
 		return;
@@ -2116,7 +2116,7 @@ static char *handle_path_level_conflicts
 	 * to ensure that's the case.
 	 */
 	collision_ent = collision_find_entry(collisions, new_path);
-	if (collision_ent == NULL)
+	if (!collision_ent)
 		BUG("collision_ent is NULL");
 
 	/*
@@ -2996,7 +2996,7 @@ static void final_cleanup_rename(struct
 	const struct rename *re;
 	int i;
 
-	if (rename == NULL)
+	if (!rename)
 		return;
 
 	for (i = 0; i < rename->nr; i++) {
@@ -3605,7 +3605,7 @@ static int merge_recursive_internal(stru
 	}
 
 	merged_merge_bases = pop_commit(&merge_bases);
-	if (merged_merge_bases == NULL) {
+	if (!merged_merge_bases) {
 		/* if there is no common ancestor, use an empty tree */
 		struct tree *tree;
 
diff -u -p a/object-file.c b/object-file.c
--- a/object-file.c
+++ b/object-file.c
@@ -1728,7 +1728,7 @@ void *read_object_file_extended(struct r
 		die(_("loose object %s (stored in %s) is corrupt"),
 		    oid_to_hex(repl), path);
 
-	if ((p = has_packed_and_bad(r, repl)) != NULL)
+	if ((p = has_packed_and_bad(r, repl)))
 		die(_("packed object %s (stored in %s) is corrupt"),
 		    oid_to_hex(repl), p->pack_name);
 	obj_read_unlock();
diff -u -p a/pack-bitmap.c b/pack-bitmap.c
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -111,7 +111,7 @@ static struct ewah_bitmap *lookup_stored
 	struct ewah_bitmap *parent;
 	struct ewah_bitmap *composed;
 
-	if (st->xor == NULL)
+	if (!st->xor)
 		return st->root;
 
 	composed = ewah_pool_new();
@@ -279,7 +279,7 @@ static int load_bitmap_entries_v1(struct
 		if (xor_offset > 0) {
 			xor_bitmap = recent_bitmaps[(i - xor_offset) % MAX_XOR_OFFSET];
 
-			if (xor_bitmap == NULL)
+			if (!xor_bitmap)
 				return error("Invalid XOR offset in bitmap pack index");
 		}
 
@@ -728,7 +728,7 @@ static int add_commit_to_bitmap(struct b
 	if (!or_with)
 		return 0;
 
-	if (*base == NULL)
+	if (!*base)
 		*base = ewah_to_bitmap(or_with);
 	else
 		bitmap_or_ewah(*base, or_with);
@@ -771,7 +771,7 @@ static struct bitmap *find_objects(struc
 	 * Best case scenario: We found bitmaps for all the roots,
 	 * so the resulting `or` bitmap has the full reachability analysis
 	 */
-	if (not_mapped == NULL)
+	if (!not_mapped)
 		return base;
 
 	roots = not_mapped;
@@ -805,7 +805,7 @@ static struct bitmap *find_objects(struc
 		struct include_data incdata;
 		struct bitmap_show_data show_data;
 
-		if (base == NULL)
+		if (!base)
 			base = bitmap_new();
 
 		incdata.bitmap_git = bitmap_git;
@@ -1299,7 +1299,7 @@ struct bitmap_index *prepare_bitmap_walk
 		reset_revision_walk();
 		revs->ignore_missing_links = 0;
 
-		if (haves_bitmap == NULL)
+		if (!haves_bitmap)
 			BUG("failed to perform bitmap walk");
 	}
 
@@ -1698,7 +1698,7 @@ void test_bitmap_walk(struct rev_info *r
 		result = ewah_to_bitmap(bm);
 	}
 
-	if (result == NULL)
+	if (!result)
 		die("Commit %s doesn't have an indexed bitmap", oid_to_hex(&root->oid));
 
 	revs->tag_objects = 1;
diff -u -p a/packfile.c b/packfile.c
--- a/packfile.c
+++ b/packfile.c
@@ -116,7 +116,7 @@ int load_idx(const char *path, const uns
 
 	if (idx_size < 4 * 256 + hashsz + hashsz)
 		return error("index file %s is too small", path);
-	if (idx_map == NULL)
+	if (!idx_map)
 		return error("empty data");
 
 	if (hdr->idx_signature == htonl(PACK_IDX_SIGNATURE)) {
diff -u -p a/path.c b/path.c
--- a/path.c
+++ b/path.c
@@ -733,7 +733,7 @@ char *interpolate_path(const char *path,
 	struct strbuf user_path = STRBUF_INIT;
 	const char *to_copy = path;
 
-	if (path == NULL)
+	if (!path)
 		goto return_null;
 
 	if (skip_prefix(path, "%(prefix)/", &path))
diff -u -p a/prio-queue.c b/prio-queue.c
--- a/prio-queue.c
+++ b/prio-queue.c
@@ -19,7 +19,7 @@ void prio_queue_reverse(struct prio_queu
 {
 	int i, j;
 
-	if (queue->compare != NULL)
+	if (queue->compare)
 		BUG("prio_queue_reverse() on non-LIFO queue");
 	for (i = 0; i < (j = (queue->nr - 1) - i); i++)
 		swap(queue, i, j);
diff -u -p a/promisor-remote.c b/promisor-remote.c
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -84,7 +84,7 @@ static void promisor_remote_move_to_tail
 					 struct promisor_remote *r,
 					 struct promisor_remote *previous)
 {
-	if (r->next == NULL)
+	if (!r->next)
 		return;
 
 	if (previous)
diff -u -p a/ref-filter.c b/ref-filter.c
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1261,7 +1261,7 @@ static void grab_date(const char *buf, s
 	 * ":" means no format is specified, and use the default.
 	 */
 	formatp = strchr(atomname, ':');
-	if (formatp != NULL) {
+	if (formatp) {
 		formatp++;
 		parse_date_format(formatp, &date_mode);
 	}
@@ -1509,7 +1509,7 @@ static void fill_missing_values(struct a
 	int i;
 	for (i = 0; i < used_atom_cnt; i++) {
 		struct atom_value *v = &val[i];
-		if (v->s == NULL)
+		if (!v->s)
 			v->s = xstrdup("");
 	}
 }
@@ -1619,7 +1619,7 @@ static const char *rstrip_ref_components
 
 	while (remaining-- > 0) {
 		char *p = strrchr(start, '/');
-		if (p == NULL) {
+		if (!p) {
 			free((char *)to_free);
 			return xstrdup("");
 		} else
diff -u -p a/refs/ref-cache.c b/refs/ref-cache.c
--- a/refs/ref-cache.c
+++ b/refs/ref-cache.c
@@ -134,7 +134,7 @@ int search_ref_dir(struct ref_dir *dir,
 	r = bsearch(&key, dir->entries, dir->nr, sizeof(*dir->entries),
 		    ref_entry_cmp_sslice);
 
-	if (r == NULL)
+	if (!r)
 		return -1;
 
 	return r - dir->entries;
diff -u -p a/reftable/stack_test.c b/reftable/stack_test.c
--- a/reftable/stack_test.c
+++ b/reftable/stack_test.c
@@ -35,7 +35,7 @@ static int count_dir_entries(const char
 	DIR *dir = opendir(dirname);
 	int len = 0;
 	struct dirent *d;
-	if (dir == NULL)
+	if (!dir)
 		return 0;
 
 	while ((d = readdir(dir))) {
diff -u -p a/reftable/tree.c b/reftable/tree.c
--- a/reftable/tree.c
+++ b/reftable/tree.c
@@ -16,7 +16,7 @@ struct tree_node *tree_search(void *key,
 			      int insert)
 {
 	int res;
-	if (*rootp == NULL) {
+	if (!*rootp) {
 		if (!insert) {
 			return NULL;
 		} else {
@@ -50,7 +50,7 @@ void infix_walk(struct tree_node *t, voi
 
 void tree_free(struct tree_node *t)
 {
-	if (t == NULL) {
+	if (!t) {
 		return;
 	}
 	if (t->left) {
diff -u -p a/reftable/writer.c b/reftable/writer.c
--- a/reftable/writer.c
+++ b/reftable/writer.c
@@ -183,7 +183,7 @@ static void writer_index_hash(struct ref
 	struct tree_node *node = tree_search(&want, &w->obj_index_tree,
 					     &obj_index_tree_node_compare, 0);
 	struct obj_index_tree_node *key = NULL;
-	if (node == NULL) {
+	if (!node) {
 		struct obj_index_tree_node empty = OBJ_INDEX_TREE_NODE_INIT;
 		key = reftable_malloc(sizeof(struct obj_index_tree_node));
 		*key = empty;
@@ -222,7 +222,7 @@ static int writer_add_record(struct reft
 
 	strbuf_reset(&w->last_key);
 	strbuf_addbuf(&w->last_key, &key);
-	if (w->block_writer == NULL) {
+	if (!w->block_writer) {
 		writer_reinit_block_writer(w, reftable_record_type(rec));
 	}
 
@@ -263,7 +263,7 @@ int reftable_writer_add_ref(struct refta
 	};
 	int err = 0;
 
-	if (ref->refname == NULL)
+	if (!ref->refname)
 		return REFTABLE_API_ERROR;
 	if (ref->update_index < w->min_update_index ||
 	    ref->update_index > w->max_update_index)
@@ -336,7 +336,7 @@ int reftable_writer_add_log(struct refta
 	if (log->value_type == REFTABLE_LOG_DELETION)
 		return reftable_writer_add_log_verbatim(w, log);
 
-	if (log->refname == NULL)
+	if (!log->refname)
 		return REFTABLE_API_ERROR;
 
 	input_log_message = log->value.update.message;
@@ -545,7 +545,7 @@ static int writer_finish_public_section(
 	uint8_t typ = 0;
 	int err = 0;
 
-	if (w->block_writer == NULL)
+	if (!w->block_writer)
 		return 0;
 
 	typ = block_writer_type(w->block_writer);
@@ -694,7 +694,7 @@ static int writer_flush_nonempty_block(s
 
 static int writer_flush_block(struct reftable_writer *w)
 {
-	if (w->block_writer == NULL)
+	if (!w->block_writer)
 		return 0;
 	if (w->block_writer->entries == 0)
 		return 0;
diff -u -p a/rerere.c b/rerere.c
--- a/rerere.c
+++ b/rerere.c
@@ -591,7 +591,7 @@ int rerere_remaining(struct repository *
 		else if (conflict_type == RESOLVED) {
 			struct string_list_item *it;
 			it = string_list_lookup(merge_rr, (const char *)e->name);
-			if (it != NULL) {
+			if (it) {
 				free_rerere_id(it);
 				it->util = RERERE_RESOLVED;
 			}
diff -u -p a/revision.c b/revision.c
--- a/revision.c
+++ b/revision.c
@@ -2833,7 +2833,7 @@ int setup_revisions(int argc, const char
 	}
 	strvec_clear(&prune_data);
 
-	if (revs->def == NULL)
+	if (!revs->def)
 		revs->def = opt ? opt->def : NULL;
 	if (opt && opt->tweak)
 		opt->tweak(revs, opt);
@@ -3652,7 +3652,7 @@ static enum rewrite_result rewrite_one_1
 			return rewrite_one_ok;
 		if (!p->parents)
 			return rewrite_one_noparents;
-		if ((p = one_relevant_parent(revs, p->parents)) == NULL)
+		if (!(p = one_relevant_parent(revs, p->parents)))
 			return rewrite_one_ok;
 		*pp = p;
 	}
diff -u -p a/setup.c b/setup.c
--- a/setup.c
+++ b/setup.c
@@ -1470,7 +1470,7 @@ int git_config_perm(const char *var, con
 	int i;
 	char *endptr;
 
-	if (value == NULL)
+	if (!value)
 		return PERM_GROUP;
 
 	if (!strcmp(value, "umask"))
diff -u -p a/sh-i18n--envsubst.c b/sh-i18n--envsubst.c
--- a/sh-i18n--envsubst.c
+++ b/sh-i18n--envsubst.c
@@ -397,7 +397,7 @@ subst_from_stdin (void)
 		  /* Substitute the variable's value from the environment.  */
 		  const char *env_value = getenv (buffer);
 
-		  if (env_value != NULL)
+		  if (env_value)
 		    fputs (env_value, stdout);
 		}
 	      else
diff -u -p a/shallow.c b/shallow.c
--- a/shallow.c
+++ b/shallow.c
@@ -560,7 +560,7 @@ static void paint_down(struct paint_info
 		else
 			c->object.flags |= SEEN;
 
-		if (*refs == NULL)
+		if (!*refs)
 			*refs = bitmap;
 		else {
 			memcpy(tmp, *refs, bitmap_size);
diff -u -p a/trailer.c b/trailer.c
--- a/trailer.c
+++ b/trailer.c
@@ -1029,7 +1029,7 @@ static FILE *create_in_place_tempfile(co
 
 	/* Create temporary file in the same directory as the original */
 	tail = strrchr(file, '/');
-	if (tail != NULL)
+	if (tail)
 		strbuf_add(&filename_template, file, tail - file + 1);
 	strbuf_addstr(&filename_template, "git-interpret-trailers-XXXXXX");
 
diff -u -p a/transport.c b/transport.c
--- a/transport.c
+++ b/transport.c
@@ -438,7 +438,7 @@ static int fetch_refs_via_pack(struct tr
 		args.self_contained_and_connected;
 	data->options.connectivity_checked = args.connectivity_checked;
 
-	if (refs == NULL)
+	if (!refs)
 		ret = -1;
 	if (report_unmatched_refs(to_fetch, nr_heads))
 		ret = -1;
diff -u -p a/wildmatch.c b/wildmatch.c
--- a/wildmatch.c
+++ b/wildmatch.c
@@ -113,7 +113,7 @@ static int dowild(const uchar *p, const
 				/* Trailing "**" matches everything.  Trailing "*" matches
 				 * only if there are no more slash characters. */
 				if (!match_slash) {
-					if (strchr((char*)text, '/') != NULL)
+					if (strchr((char *)text, '/'))
 						return WM_NOMATCH;
 				}
 				return WM_MATCH;
diff -u -p a/worktree.c b/worktree.c
--- a/worktree.c
+++ b/worktree.c
@@ -483,7 +483,7 @@ int submodule_uses_worktrees(const char
 		return 0;
 
 	d = readdir_skip_dot_and_dotdot(dir);
-	if (d != NULL)
+	if (d)
 		ret = 1;
 	closedir(dir);
 	return ret;
diff -u -p a/wrapper.c b/wrapper.c
--- a/wrapper.c
+++ b/wrapper.c
@@ -393,7 +393,7 @@ FILE *xfopen(const char *path, const cha
 FILE *xfdopen(int fd, const char *mode)
 {
 	FILE *stream = fdopen(fd, mode);
-	if (stream == NULL)
+	if (!stream)
 		die_errno("Out of memory? fdopen failed");
 	return stream;
 }
diff -u -p a/xdiff-interface.c b/xdiff-interface.c
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -159,7 +159,7 @@ int read_mmfile(mmfile_t *ptr, const cha
 
 	if (stat(filename, &st))
 		return error_errno("Could not stat %s", filename);
-	if ((f = fopen(filename, "rb")) == NULL)
+	if (!(f = fopen(filename, "rb")))
 		return error_errno("Could not open %s", filename);
 	sz = xsize_t(st.st_size);
 	ptr->ptr = xmalloc(sz ? sz : 1);
diff -u -p a/xdiff/xemit.c b/xdiff/xemit.c
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -65,7 +65,7 @@ xdchange_t *xdl_get_hunk(xdchange_t **xs
 			*xscr = xch;
 	}
 
-	if (*xscr == NULL)
+	if (!*xscr)
 		return NULL;
 
 	lxch = *xscr;
diff -u -p a/xdiff/xprepare.c b/xdiff/xprepare.c
--- a/xdiff/xprepare.c
+++ b/xdiff/xprepare.c
@@ -188,7 +188,7 @@ static int xdl_prepare_ctx(unsigned int
 	memset(rhash, 0, hsize * sizeof(xrecord_t *));
 
 	nrec = 0;
-	if ((cur = blk = xdl_mmfile_first(mf, &bsize)) != NULL) {
+	if ((cur = blk = xdl_mmfile_first(mf, &bsize))) {
 		for (top = blk + bsize; cur < top; ) {
 			prev = cur;
 			hav = xdl_hash_record(&cur, top, xpp->flags);
diff -u -p a/xdiff/xutils.c b/xdiff/xutils.c
--- a/xdiff/xutils.c
+++ b/xdiff/xutils.c
@@ -122,7 +122,7 @@ long xdl_guess_lines(mmfile_t *mf, long
 	long nl = 0, size, tsize = 0;
 	char const *data, *cur, *top;
 
-	if ((cur = data = xdl_mmfile_first(mf, &size)) != NULL) {
+	if ((cur = data = xdl_mmfile_first(mf, &size))) {
 		for (top = data + size; nl < sample && cur < top; ) {
 			nl++;
 			if (!(cur = memchr(cur, '\n', top - cur)))

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

[Elia Pinto]

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

>


>
> 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;
>
>         /*
> diff -u -p a/compat/mingw.c b/compat/mingw.c
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -1060,7 +1060,7 @@ char *mingw_mktemp(char *template)
>  int mkstemp(char *template)
>  {
>         char *filename = mktemp(template);
> -       if (filename == NULL)
> +       if (!filename)
>                 return -1;
>         return open(filename, O_RDWR | O_CREAT, 0600);
>  }
> @@ -2332,7 +2332,7 @@ int setitimer(int type, struct itimerval
>         static const struct timeval zero;
>         static int atexit_done;
>
> -       if (out != NULL)
> +       if (out)
>                 return errno = EINVAL,
>                         error("setitimer param 3 != NULL not implemented");
>         if (!is_timeval_eq(&in->it_interval, &zero) &&
> @@ -2361,7 +2361,7 @@ int sigaction(int sig, struct sigaction
>         if (sig != SIGALRM)
>                 return errno = EINVAL,
>                         error("sigaction only implemented for SIGALRM");
> -       if (out != NULL)
> +       if (out)
>                 return errno = EINVAL,
>                         error("sigaction: param 3 != NULL not implemented");
>
> diff -u -p a/compat/mkdir.c b/compat/mkdir.c
> --- a/compat/mkdir.c
> +++ b/compat/mkdir.c
> @@ -9,7 +9,7 @@ int compat_mkdir_wo_trailing_slash(const
>         size_t len = strlen(dir);
>
>         if (len && dir[len-1] == '/') {
> -               if ((tmp_dir = strdup(dir)) == NULL)
> +               if (!(tmp_dir = strdup(dir)))
>                         return -1;
>                 tmp_dir[len-1] = '\0';
>         }
> diff -u -p a/compat/mmap.c b/compat/mmap.c
> --- a/compat/mmap.c
> +++ b/compat/mmap.c
> @@ -13,7 +13,7 @@ void *git_mmap(void *start, size_t lengt
>         }
>
>         start = malloc(length);
> -       if (start == NULL) {
> +       if (!start) {
>                 errno = ENOMEM;
>                 return MAP_FAILED;
>         }
> diff -u -p a/compat/mingw.c b/compat/mingw.c
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -1060,7 +1060,7 @@ char *mingw_mktemp(char *template)
>  int mkstemp(char *template)
>  {
>         char *filename = mktemp(template);
> -       if (filename == NULL)
> +       if (!filename)
>                 return -1;
>         return open(filename, O_RDWR | O_CREAT, 0600);
>  }
> @@ -2332,7 +2332,7 @@ int setitimer(int type, struct itimerval
>         static const struct timeval zero;
>         static int atexit_done;
>
> -       if (out != NULL)
> +       if (out)
>                 return errno = EINVAL,
>                         error("setitimer param 3 != NULL not implemented");
>         if (!is_timeval_eq(&in->it_interval, &zero) &&
> @@ -2361,7 +2361,7 @@ int sigaction(int sig, struct sigaction
>         if (sig != SIGALRM)
>                 return errno = EINVAL,
>                         error("sigaction only implemented for SIGALRM");
> -       if (out != NULL)
> +       if (out)
>                 return errno = EINVAL,
>                         error("sigaction: param 3 != NULL not implemented");
>
> diff -u -p a/config.c b/config.c
> --- a/config.c
> +++ b/config.c
> @@ -3190,7 +3190,7 @@ int git_config_set_multivar_in_file_gent
>                         goto out_free;
>                 }
>                 /* if nothing to unset, error out */
> -               if (value == NULL) {
> +               if (!value) {
>                         ret = CONFIG_NOTHING_SET;
>                         goto out_free;
>                 }
> @@ -3206,7 +3206,7 @@ int git_config_set_multivar_in_file_gent
>                 int i, new_line = 0;
>                 struct config_options opts;
>
> -               if (value_pattern == NULL)
> +               if (!value_pattern)
>                         store.value_pattern = NULL;
>                 else if (value_pattern == CONFIG_REGEX_NONE)
>                         store.value_pattern = CONFIG_REGEX_NONE;
> @@ -3346,7 +3346,7 @@ int git_config_set_multivar_in_file_gent
>                 }
>
>                 /* write the pair (value == NULL means unset) */
> -               if (value != NULL) {
> +               if (value) {
>                         if (!store.section_seen) {
>                                 if (write_section(fd, key, &store) < 0)
>                                         goto write_err_out;
> @@ -3567,7 +3567,7 @@ static int git_config_copy_or_rename_sec
>                         offset = section_name_match(&buf[i], old_name);
>                         if (offset > 0) {
>                                 ret++;
> -                               if (new_name == NULL) {
> +                               if (!new_name) {
>                                         remove = 1;
>                                         continue;
>                                 }
> diff -u -p a/daemon.c b/daemon.c
> --- a/daemon.c
> +++ b/daemon.c
> @@ -447,7 +447,7 @@ static void copy_to_log(int fd)
>         FILE *fp;
>
>         fp = fdopen(fd, "r");
> -       if (fp == NULL) {
> +       if (!fp) {
>                 logerror("fdopen of error channel failed");
>                 close(fd);
>                 return;
> diff -u -p a/dir.c b/dir.c
> --- a/dir.c
> +++ b/dir.c
> @@ -3054,7 +3054,7 @@ char *git_url_basename(const char *repo,
>          * Skip scheme.
>          */
>         start = strstr(repo, "://");
> -       if (start == NULL)
> +       if (!start)
>                 start = repo;
>         else
>                 start += 3;
> diff -u -p a/ewah/bitmap.c b/ewah/bitmap.c
> --- a/ewah/bitmap.c
> +++ b/ewah/bitmap.c
> @@ -223,7 +223,7 @@ void bitmap_reset(struct bitmap *bitmap)
>
>  void bitmap_free(struct bitmap *bitmap)
>  {
> -       if (bitmap == NULL)
> +       if (!bitmap)
>                 return;
>
>         free(bitmap->words);
> diff -u -p a/ewah/ewah_bitmap.c b/ewah/ewah_bitmap.c
> --- a/ewah/ewah_bitmap.c
> +++ b/ewah/ewah_bitmap.c
> @@ -451,7 +451,7 @@ struct ewah_bitmap *ewah_pool_new(void)
>
>  void ewah_pool_free(struct ewah_bitmap *self)
>  {
> -       if (self == NULL)
> +       if (!self)
>                 return;
>
>         if (bitmap_pool_size == BITMAP_POOL_MAX ||
> diff -u -p a/http-fetch.c b/http-fetch.c
> --- a/http-fetch.c
> +++ b/http-fetch.c
> @@ -55,7 +55,7 @@ static void fetch_single_packfile(struct
>         http_init(NULL, url, 0);
>
>         preq = new_direct_http_pack_request(packfile_hash->hash, xstrdup(url));
> -       if (preq == NULL)
> +       if (!preq)
>                 die("couldn't create http pack request");
>         preq->slot->results = &results;
>         preq->index_pack_args = index_pack_args;
> diff -u -p a/http-push.c b/http-push.c
> --- a/http-push.c
> +++ b/http-push.c
> @@ -253,7 +253,7 @@ static void start_fetch_loose(struct tra
>         struct http_object_request *obj_req;
>
>         obj_req = new_http_object_request(repo->url, &request->obj->oid);
> -       if (obj_req == NULL) {
> +       if (!obj_req) {
>                 request->state = ABORTED;
>                 return;
>         }
> @@ -318,7 +318,7 @@ static void start_fetch_packed(struct tr
>         fprintf(stderr, " which contains %s\n", oid_to_hex(&request->obj->oid));
>
>         preq = new_http_pack_request(target->hash, repo->url);
> -       if (preq == NULL) {
> +       if (!preq) {
>                 repo->can_update_info_refs = 0;
>                 return;
>         }
> @@ -520,7 +520,7 @@ static void finish_request(struct transf
>         /* Keep locks active */
>         check_locks();
>
> -       if (request->headers != NULL)
> +       if (request->headers)
>                 curl_slist_free_all(request->headers);
>
>         /* URL is reused for MOVE after PUT and used during FETCH */
> @@ -783,7 +783,7 @@ xml_start_tag(void *userData, const char
>         const char *c = strchr(name, ':');
>         int old_namelen, new_len;
>
> -       if (c == NULL)
> +       if (!c)
>                 c = name;
>         else
>                 c++;
> @@ -811,7 +811,7 @@ xml_end_tag(void *userData, const char *
>
>         ctx->userFunc(ctx, 1);
>
> -       if (c == NULL)
> +       if (!c)
>                 c = name;
>         else
>                 c++;
> @@ -1893,7 +1893,7 @@ int cmd_main(int argc, const char **argv
>
>                 /* Lock remote branch ref */
>                 ref_lock = lock_remote(ref->name, LOCK_TIME);
> -               if (ref_lock == NULL) {
> +               if (!ref_lock) {
>                         fprintf(stderr, "Unable to lock remote branch %s\n",
>                                 ref->name);
>                         if (helper_status)
> diff -u -p a/http-walker.c b/http-walker.c
> --- a/http-walker.c
> +++ b/http-walker.c
> @@ -59,7 +59,7 @@ static void start_object_request(struct
>         struct http_object_request *req;
>
>         req = new_http_object_request(obj_req->repo->base, &obj_req->oid);
> -       if (req == NULL) {
> +       if (!req) {
>                 obj_req->state = ABORTED;
>                 return;
>         }
> @@ -106,7 +106,7 @@ static void process_object_response(void
>         /* Use alternates if necessary */
>         if (missing_target(obj_req->req)) {
>                 fetch_alternates(walker, alt->base);
> -               if (obj_req->repo->next != NULL) {
> +               if (obj_req->repo->next) {
>                         obj_req->repo =
>                                 obj_req->repo->next;
>                         release_http_object_request(obj_req->req);
> @@ -225,12 +225,12 @@ static void process_alternates_response(
>                                          alt_req->url->buf);
>                         active_requests++;
>                         slot->in_use = 1;
> -                       if (slot->finished != NULL)
> +                       if (slot->finished)
>                                 (*slot->finished) = 0;
>                         if (!start_active_slot(slot)) {
>                                 cdata->got_alternates = -1;
>                                 slot->in_use = 0;
> -                               if (slot->finished != NULL)
> +                               if (slot->finished)
>                                         (*slot->finished) = 1;
>                         }
>                         return;
> @@ -443,7 +443,7 @@ static int http_fetch_pack(struct walker
>         }
>
>         preq = new_http_pack_request(target->hash, repo->base);
> -       if (preq == NULL)
> +       if (!preq)
>                 goto abort;
>         preq->slot->results = &results;
>
> @@ -489,11 +489,11 @@ static int fetch_object(struct walker *w
>                 if (hasheq(obj_req->oid.hash, hash))
>                         break;
>         }
> -       if (obj_req == NULL)
> +       if (!obj_req)
>                 return error("Couldn't find request for %s in the queue", hex);
>
>         if (has_object_file(&obj_req->oid)) {
> -               if (obj_req->req != NULL)
> +               if (obj_req->req)
>                         abort_http_object_request(obj_req->req);
>                 abort_object_request(obj_req);
>                 return 0;
> diff -u -p a/http.c b/http.c
> --- a/http.c
> +++ b/http.c
> @@ -197,11 +197,11 @@ static void finish_active_slot(struct ac
>         closedown_active_slot(slot);
>         curl_easy_getinfo(slot->curl, CURLINFO_HTTP_CODE, &slot->http_code);
>
> -       if (slot->finished != NULL)
> +       if (slot->finished)
>                 (*slot->finished) = 1;
>
>         /* Store slot results so they can be read after the slot is reused */
> -       if (slot->results != NULL) {
> +       if (slot->results) {
>                 slot->results->curl_result = slot->curl_result;
>                 slot->results->http_code = slot->http_code;
>                 curl_easy_getinfo(slot->curl, CURLINFO_HTTPAUTH_AVAIL,
> @@ -212,7 +212,7 @@ static void finish_active_slot(struct ac
>         }
>
>         /* Run callback if appropriate */
> -       if (slot->callback_func != NULL)
> +       if (slot->callback_func)
>                 slot->callback_func(slot->callback_data);
>  }
>
> @@ -234,7 +234,7 @@ static void process_curl_messages(void)
>                         while (slot != NULL &&
>                                slot->curl != curl_message->easy_handle)
>                                 slot = slot->next;
> -                       if (slot != NULL) {
> +                       if (slot) {
>                                 xmulti_remove_handle(slot);
>                                 slot->curl_result = curl_result;
>                                 finish_active_slot(slot);
> @@ -838,16 +838,16 @@ static CURL *get_curl_handle(void)
>                 curl_easy_setopt(result, CURLOPT_SSL_CIPHER_LIST,
>                                 ssl_cipherlist);
>
> -       if (ssl_cert != NULL)
> +       if (ssl_cert)
>                 curl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);
>         if (has_cert_password())
>                 curl_easy_setopt(result, CURLOPT_KEYPASSWD, cert_auth.password);
> -       if (ssl_key != NULL)
> +       if (ssl_key)
>                 curl_easy_setopt(result, CURLOPT_SSLKEY, ssl_key);
> -       if (ssl_capath != NULL)
> +       if (ssl_capath)
>                 curl_easy_setopt(result, CURLOPT_CAPATH, ssl_capath);
>  #ifdef GIT_CURL_HAVE_CURLOPT_PINNEDPUBLICKEY
> -       if (ssl_pinnedkey != NULL)
> +       if (ssl_pinnedkey)
>                 curl_easy_setopt(result, CURLOPT_PINNEDPUBLICKEY, ssl_pinnedkey);
>  #endif
>         if (http_ssl_backend && !strcmp("schannel", http_ssl_backend) &&
> @@ -857,10 +857,10 @@ static CURL *get_curl_handle(void)
>                 curl_easy_setopt(result, CURLOPT_PROXY_CAINFO, NULL);
>  #endif
>         } else if (ssl_cainfo != NULL || http_proxy_ssl_ca_info != NULL) {
> -               if (ssl_cainfo != NULL)
> +               if (ssl_cainfo)
>                         curl_easy_setopt(result, CURLOPT_CAINFO, ssl_cainfo);
>  #ifdef GIT_CURL_HAVE_CURLOPT_PROXY_CAINFO
> -               if (http_proxy_ssl_ca_info != NULL)
> +               if (http_proxy_ssl_ca_info)
>                         curl_easy_setopt(result, CURLOPT_PROXY_CAINFO, http_proxy_ssl_ca_info);
>  #endif
>         }
> @@ -1050,7 +1050,7 @@ void http_init(struct remote *remote, co
>
>         {
>                 char *http_max_requests = getenv("GIT_HTTP_MAX_REQUESTS");
> -               if (http_max_requests != NULL)
> +               if (http_max_requests)
>                         max_requests = atoi(http_max_requests);
>         }
>
> @@ -1069,10 +1069,10 @@ void http_init(struct remote *remote, co
>         set_from_env(&user_agent, "GIT_HTTP_USER_AGENT");
>
>         low_speed_limit = getenv("GIT_HTTP_LOW_SPEED_LIMIT");
> -       if (low_speed_limit != NULL)
> +       if (low_speed_limit)
>                 curl_low_speed_limit = strtol(low_speed_limit, NULL, 10);
>         low_speed_time = getenv("GIT_HTTP_LOW_SPEED_TIME");
> -       if (low_speed_time != NULL)
> +       if (low_speed_time)
>                 curl_low_speed_time = strtol(low_speed_time, NULL, 10);
>
>         if (curl_ssl_verify == -1)
> @@ -1109,7 +1109,7 @@ void http_cleanup(void)
>
>         while (slot != NULL) {
>                 struct active_request_slot *next = slot->next;
> -               if (slot->curl != NULL) {
> +               if (slot->curl) {
>                         xmulti_remove_handle(slot);
>                         curl_easy_cleanup(slot->curl);
>                 }
> @@ -1147,13 +1147,13 @@ void http_cleanup(void)
>         free((void *)http_proxy_authmethod);
>         http_proxy_authmethod = NULL;
>
> -       if (cert_auth.password != NULL) {
> +       if (cert_auth.password) {
>                 memset(cert_auth.password, 0, strlen(cert_auth.password));
>                 FREE_AND_NULL(cert_auth.password);
>         }
>         ssl_cert_password_required = 0;
>
> -       if (proxy_cert_auth.password != NULL) {
> +       if (proxy_cert_auth.password) {
>                 memset(proxy_cert_auth.password, 0, strlen(proxy_cert_auth.password));
>                 FREE_AND_NULL(proxy_cert_auth.password);
>         }
> @@ -1179,14 +1179,14 @@ struct active_request_slot *get_active_s
>         while (slot != NULL && slot->in_use)
>                 slot = slot->next;
>
> -       if (slot == NULL) {
> +       if (!slot) {
>                 newslot = xmalloc(sizeof(*newslot));
>                 newslot->curl = NULL;
>                 newslot->in_use = 0;
>                 newslot->next = NULL;
>
>                 slot = active_queue_head;
> -               if (slot == NULL) {
> +               if (!slot) {
>                         active_queue_head = newslot;
>                 } else {
>                         while (slot->next != NULL)
> @@ -1196,7 +1196,7 @@ struct active_request_slot *get_active_s
>                 slot = newslot;
>         }
>
> -       if (slot->curl == NULL) {
> +       if (!slot->curl) {
>                 slot->curl = curl_easy_duphandle(curl_default);
>                 curl_session_count++;
>         }
> @@ -1768,7 +1768,7 @@ static int http_request(const char *url,
>         slot = get_active_slot();
>         curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);
>
> -       if (result == NULL) {
> +       if (!result) {
>                 curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1);
>         } else {
>                 curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
> @@ -2100,7 +2100,7 @@ cleanup:
>
>  void release_http_pack_request(struct http_pack_request *preq)
>  {
> -       if (preq->packfile != NULL) {
> +       if (preq->packfile) {
>                 fclose(preq->packfile);
>                 preq->packfile = NULL;
>         }
> @@ -2391,7 +2391,7 @@ abort:
>
>  void process_http_object_request(struct http_object_request *freq)
>  {
> -       if (freq->slot == NULL)
> +       if (!freq->slot)
>                 return;
>         freq->curl_result = freq->slot->curl_result;
>         freq->http_code = freq->slot->http_code;
> @@ -2448,7 +2448,7 @@ void release_http_object_request(struct
>                 freq->localfile = -1;
>         }
>         FREE_AND_NULL(freq->url);
> -       if (freq->slot != NULL) {
> +       if (freq->slot) {
>                 freq->slot->callback_func = NULL;
>                 freq->slot->callback_data = NULL;
>                 release_active_slot(freq->slot);
> diff -u -p a/kwset.c b/kwset.c
> --- a/kwset.c
> +++ b/kwset.c
> @@ -477,7 +477,7 @@ kwsprep (kwset_t kws)
>         next[i] = NULL;
>        treenext(kwset->trie->links, next);
>
> -      if ((trans = kwset->trans) != NULL)
> +      if ((trans = kwset->trans))
>         for (i = 0; i < NCHAR; ++i)
>           kwset->next[i] = next[U(trans[i])];
>        else
> @@ -485,7 +485,7 @@ kwsprep (kwset_t kws)
>      }
>
>    /* Fix things up for any translation table. */
> -  if ((trans = kwset->trans) != NULL)
> +  if ((trans = kwset->trans))
>      for (i = 0; i < NCHAR; ++i)
>        kwset->delta[i] = delta[U(trans[i])];
>    else
> diff -u -p a/ll-merge.c b/ll-merge.c
> --- a/ll-merge.c
> +++ b/ll-merge.c
> @@ -207,7 +207,7 @@ static enum ll_merge_result ll_ext_merge
>         dict[4].placeholder = "P"; dict[4].value = path_sq.buf;
>         dict[5].placeholder = NULL; dict[5].value = NULL;
>
> -       if (fn->cmdline == NULL)
> +       if (!fn->cmdline)
>                 die("custom merge driver %s lacks command line.", fn->name);
>
>         result->ptr = NULL;
> diff -u -p a/log-tree.c b/log-tree.c
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -88,7 +88,7 @@ static int match_ref_pattern(const char
>                              const struct string_list_item *item)
>  {
>         int matched = 0;
> -       if (item->util == NULL) {
> +       if (!item->util) {
>                 if (!wildmatch(item->string, refname, 0))
>                         matched = 1;
>         } else {
> diff -u -p a/mailinfo.c b/mailinfo.c
> --- a/mailinfo.c
> +++ b/mailinfo.c
> @@ -698,7 +698,7 @@ static int is_scissors_line(const char *
>                         continue;
>                 }
>                 last_nonblank = c;
> -               if (first_nonblank == NULL)
> +               if (!first_nonblank)
>                         first_nonblank = c;
>                 if (*c == '-') {
>                         in_perforation = 1;
> @@ -1094,7 +1094,7 @@ static void handle_body(struct mailinfo
>                          */
>                         lines = strbuf_split(line, '\n');
>                         for (it = lines; (sb = *it); it++) {
> -                               if (*(it + 1) == NULL) /* The last line */
> +                               if (!*(it + 1)) /* The last line */
>                                         if (sb->buf[sb->len - 1] != '\n') {
>                                                 /* Partial line, save it for later. */
>                                                 strbuf_addbuf(&prev, sb);
> diff -u -p a/mailmap.c b/mailmap.c
> --- a/mailmap.c
> +++ b/mailmap.c
> @@ -77,7 +77,7 @@ static void add_mapping(struct string_li
>         struct mailmap_entry *me;
>         struct string_list_item *item;
>
> -       if (old_email == NULL) {
> +       if (!old_email) {
>                 old_email = new_email;
>                 new_email = NULL;
>         }
> @@ -92,7 +92,7 @@ static void add_mapping(struct string_li
>                 item->util = me;
>         }
>
> -       if (old_name == NULL) {
> +       if (!old_name) {
>                 debug_mm("mailmap: adding (simple) entry for '%s'\n", old_email);
>
>                 /* Replace current name and new email for simple entry */
> @@ -123,9 +123,9 @@ static char *parse_name_and_email(char *
>         char *left, *right, *nstart, *nend;
>         *name = *email = NULL;
>
> -       if ((left = strchr(buffer, '<')) == NULL)
> +       if (!(left = strchr(buffer, '<')))
>                 return NULL;
> -       if ((right = strchr(left+1, '>')) == NULL)
> +       if (!(right = strchr(left + 1, '>')))
>                 return NULL;
>         if (!allow_empty_email && (left+1 == right))
>                 return NULL;
> @@ -153,7 +153,7 @@ static void read_mailmap_line(struct str
>         if (buffer[0] == '#')
>                 return;
>
> -       if ((name2 = parse_name_and_email(buffer, &name1, &email1, 0)) != NULL)
> +       if ((name2 = parse_name_and_email(buffer, &name1, &email1, 0)))
>                 parse_name_and_email(name2, &name2, &email2, 1);
>
>         if (email1)
> @@ -320,7 +320,7 @@ int map_user(struct string_list *map,
>                  (int)*emaillen, debug_str(*email));
>
>         item = lookup_prefix(map, *email, *emaillen);
> -       if (item != NULL) {
> +       if (item) {
>                 me = (struct mailmap_entry *)item->util;
>                 if (me->namemap.nr) {
>                         /*
> @@ -334,7 +334,7 @@ int map_user(struct string_list *map,
>                                 item = subitem;
>                 }
>         }
> -       if (item != NULL) {
> +       if (item) {
>                 struct mailmap_info *mi = (struct mailmap_info *)item->util;
>                 if (mi->name == NULL && mi->email == NULL) {
>                         debug_mm("map_user:  -- (no simple mapping)\n");
> diff -u -p a/merge-ort.c b/merge-ort.c
> --- a/merge-ort.c
> +++ b/merge-ort.c
> @@ -2068,7 +2068,7 @@ static char *handle_path_level_conflicts
>          * to ensure that's the case.
>          */
>         c_info = strmap_get(collisions, new_path);
> -       if (c_info == NULL)
> +       if (!c_info)
>                 BUG("c_info is NULL");
>
>         /*
> @@ -4640,7 +4640,7 @@ static void merge_ort_internal(struct me
>         }
>
>         merged_merge_bases = pop_commit(&merge_bases);
> -       if (merged_merge_bases == NULL) {
> +       if (!merged_merge_bases) {
>                 /* if there is no common ancestor, use an empty tree */
>                 struct tree *tree;
>
> diff -u -p a/merge-recursive.c b/merge-recursive.c
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -82,7 +82,7 @@ static struct dir_rename_entry *dir_rena
>  {
>         struct dir_rename_entry key;
>
> -       if (dir == NULL)
> +       if (!dir)
>                 return NULL;
>         hashmap_entry_init(&key.ent, strhash(dir));
>         key.dir = dir;
> @@ -1990,14 +1990,14 @@ static void get_renamed_dir_portion(cons
>          * renamed means the root directory can never be renamed -- because
>          * the root directory always exists).
>          */
> -       if (end_of_old == NULL)
> +       if (!end_of_old)
>                 return; /* Note: *old_dir and *new_dir are still NULL */
>
>         /*
>          * If new_path contains no directory (end_of_new is NULL), then we
>          * have a rename of old_path's directory to the root directory.
>          */
> -       if (end_of_new == NULL) {
> +       if (!end_of_new) {
>                 *old_dir = xstrndup(old_path, end_of_old - old_path);
>                 *new_dir = xstrdup("");
>                 return;
> @@ -2116,7 +2116,7 @@ static char *handle_path_level_conflicts
>          * to ensure that's the case.
>          */
>         collision_ent = collision_find_entry(collisions, new_path);
> -       if (collision_ent == NULL)
> +       if (!collision_ent)
>                 BUG("collision_ent is NULL");
>
>         /*
> @@ -2996,7 +2996,7 @@ static void final_cleanup_rename(struct
>         const struct rename *re;
>         int i;
>
> -       if (rename == NULL)
> +       if (!rename)
>                 return;
>
>         for (i = 0; i < rename->nr; i++) {
> @@ -3605,7 +3605,7 @@ static int merge_recursive_internal(stru
>         }
>
>         merged_merge_bases = pop_commit(&merge_bases);
> -       if (merged_merge_bases == NULL) {
> +       if (!merged_merge_bases) {
>                 /* if there is no common ancestor, use an empty tree */
>                 struct tree *tree;
>
> diff -u -p a/object-file.c b/object-file.c
> --- a/object-file.c
> +++ b/object-file.c
> @@ -1728,7 +1728,7 @@ void *read_object_file_extended(struct r
>                 die(_("loose object %s (stored in %s) is corrupt"),
>                     oid_to_hex(repl), path);
>
> -       if ((p = has_packed_and_bad(r, repl)) != NULL)
> +       if ((p = has_packed_and_bad(r, repl)))
>                 die(_("packed object %s (stored in %s) is corrupt"),
>                     oid_to_hex(repl), p->pack_name);
>         obj_read_unlock();
> diff -u -p a/pack-bitmap.c b/pack-bitmap.c
> --- a/pack-bitmap.c
> +++ b/pack-bitmap.c
> @@ -111,7 +111,7 @@ static struct ewah_bitmap *lookup_stored
>         struct ewah_bitmap *parent;
>         struct ewah_bitmap *composed;
>
> -       if (st->xor == NULL)
> +       if (!st->xor)
>                 return st->root;
>
>         composed = ewah_pool_new();
> @@ -279,7 +279,7 @@ static int load_bitmap_entries_v1(struct
>                 if (xor_offset > 0) {
>                         xor_bitmap = recent_bitmaps[(i - xor_offset) % MAX_XOR_OFFSET];
>
> -                       if (xor_bitmap == NULL)
> +                       if (!xor_bitmap)
>                                 return error("Invalid XOR offset in bitmap pack index");
>                 }
>
> @@ -728,7 +728,7 @@ static int add_commit_to_bitmap(struct b
>         if (!or_with)
>                 return 0;
>
> -       if (*base == NULL)
> +       if (!*base)
>                 *base = ewah_to_bitmap(or_with);
>         else
>                 bitmap_or_ewah(*base, or_with);
> @@ -771,7 +771,7 @@ static struct bitmap *find_objects(struc
>          * Best case scenario: We found bitmaps for all the roots,
>          * so the resulting `or` bitmap has the full reachability analysis
>          */
> -       if (not_mapped == NULL)
> +       if (!not_mapped)
>                 return base;
>
>         roots = not_mapped;
> @@ -805,7 +805,7 @@ static struct bitmap *find_objects(struc
>                 struct include_data incdata;
>                 struct bitmap_show_data show_data;
>
> -               if (base == NULL)
> +               if (!base)
>                         base = bitmap_new();
>
>                 incdata.bitmap_git = bitmap_git;
> @@ -1299,7 +1299,7 @@ struct bitmap_index *prepare_bitmap_walk
>                 reset_revision_walk();
>                 revs->ignore_missing_links = 0;
>
> -               if (haves_bitmap == NULL)
> +               if (!haves_bitmap)
>                         BUG("failed to perform bitmap walk");
>         }
>
> @@ -1698,7 +1698,7 @@ void test_bitmap_walk(struct rev_info *r
>                 result = ewah_to_bitmap(bm);
>         }
>
> -       if (result == NULL)
> +       if (!result)
>                 die("Commit %s doesn't have an indexed bitmap", oid_to_hex(&root->oid));
>
>         revs->tag_objects = 1;
> diff -u -p a/packfile.c b/packfile.c
> --- a/packfile.c
> +++ b/packfile.c
> @@ -116,7 +116,7 @@ int load_idx(const char *path, const uns
>
>         if (idx_size < 4 * 256 + hashsz + hashsz)
>                 return error("index file %s is too small", path);
> -       if (idx_map == NULL)
> +       if (!idx_map)
>                 return error("empty data");
>
>         if (hdr->idx_signature == htonl(PACK_IDX_SIGNATURE)) {
> diff -u -p a/path.c b/path.c
> --- a/path.c
> +++ b/path.c
> @@ -733,7 +733,7 @@ char *interpolate_path(const char *path,
>         struct strbuf user_path = STRBUF_INIT;
>         const char *to_copy = path;
>
> -       if (path == NULL)
> +       if (!path)
>                 goto return_null;
>
>         if (skip_prefix(path, "%(prefix)/", &path))
> diff -u -p a/prio-queue.c b/prio-queue.c
> --- a/prio-queue.c
> +++ b/prio-queue.c
> @@ -19,7 +19,7 @@ void prio_queue_reverse(struct prio_queu
>  {
>         int i, j;
>
> -       if (queue->compare != NULL)
> +       if (queue->compare)
>                 BUG("prio_queue_reverse() on non-LIFO queue");
>         for (i = 0; i < (j = (queue->nr - 1) - i); i++)
>                 swap(queue, i, j);
> diff -u -p a/promisor-remote.c b/promisor-remote.c
> --- a/promisor-remote.c
> +++ b/promisor-remote.c
> @@ -84,7 +84,7 @@ static void promisor_remote_move_to_tail
>                                          struct promisor_remote *r,
>                                          struct promisor_remote *previous)
>  {
> -       if (r->next == NULL)
> +       if (!r->next)
>                 return;
>
>         if (previous)
> diff -u -p a/ref-filter.c b/ref-filter.c
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1261,7 +1261,7 @@ static void grab_date(const char *buf, s
>          * ":" means no format is specified, and use the default.
>          */
>         formatp = strchr(atomname, ':');
> -       if (formatp != NULL) {
> +       if (formatp) {
>                 formatp++;
>                 parse_date_format(formatp, &date_mode);
>         }
> @@ -1509,7 +1509,7 @@ static void fill_missing_values(struct a
>         int i;
>         for (i = 0; i < used_atom_cnt; i++) {
>                 struct atom_value *v = &val[i];
> -               if (v->s == NULL)
> +               if (!v->s)
>                         v->s = xstrdup("");
>         }
>  }
> @@ -1619,7 +1619,7 @@ static const char *rstrip_ref_components
>
>         while (remaining-- > 0) {
>                 char *p = strrchr(start, '/');
> -               if (p == NULL) {
> +               if (!p) {
>                         free((char *)to_free);
>                         return xstrdup("");
>                 } else
> diff -u -p a/refs/ref-cache.c b/refs/ref-cache.c
> --- a/refs/ref-cache.c
> +++ b/refs/ref-cache.c
> @@ -134,7 +134,7 @@ int search_ref_dir(struct ref_dir *dir,
>         r = bsearch(&key, dir->entries, dir->nr, sizeof(*dir->entries),
>                     ref_entry_cmp_sslice);
>
> -       if (r == NULL)
> +       if (!r)
>                 return -1;
>
>         return r - dir->entries;
> diff -u -p a/reftable/stack_test.c b/reftable/stack_test.c
> --- a/reftable/stack_test.c
> +++ b/reftable/stack_test.c
> @@ -35,7 +35,7 @@ static int count_dir_entries(const char
>         DIR *dir = opendir(dirname);
>         int len = 0;
>         struct dirent *d;
> -       if (dir == NULL)
> +       if (!dir)
>                 return 0;
>
>         while ((d = readdir(dir))) {
> diff -u -p a/reftable/tree.c b/reftable/tree.c
> --- a/reftable/tree.c
> +++ b/reftable/tree.c
> @@ -16,7 +16,7 @@ struct tree_node *tree_search(void *key,
>                               int insert)
>  {
>         int res;
> -       if (*rootp == NULL) {
> +       if (!*rootp) {
>                 if (!insert) {
>                         return NULL;
>                 } else {
> @@ -50,7 +50,7 @@ void infix_walk(struct tree_node *t, voi
>
>  void tree_free(struct tree_node *t)
>  {
> -       if (t == NULL) {
> +       if (!t) {
>                 return;
>         }
>         if (t->left) {
> diff -u -p a/reftable/writer.c b/reftable/writer.c
> --- a/reftable/writer.c
> +++ b/reftable/writer.c
> @@ -183,7 +183,7 @@ static void writer_index_hash(struct ref
>         struct tree_node *node = tree_search(&want, &w->obj_index_tree,
>                                              &obj_index_tree_node_compare, 0);
>         struct obj_index_tree_node *key = NULL;
> -       if (node == NULL) {
> +       if (!node) {
>                 struct obj_index_tree_node empty = OBJ_INDEX_TREE_NODE_INIT;
>                 key = reftable_malloc(sizeof(struct obj_index_tree_node));
>                 *key = empty;
> @@ -222,7 +222,7 @@ static int writer_add_record(struct reft
>
>         strbuf_reset(&w->last_key);
>         strbuf_addbuf(&w->last_key, &key);
> -       if (w->block_writer == NULL) {
> +       if (!w->block_writer) {
>                 writer_reinit_block_writer(w, reftable_record_type(rec));
>         }
>
> @@ -263,7 +263,7 @@ int reftable_writer_add_ref(struct refta
>         };
>         int err = 0;
>
> -       if (ref->refname == NULL)
> +       if (!ref->refname)
>                 return REFTABLE_API_ERROR;
>         if (ref->update_index < w->min_update_index ||
>             ref->update_index > w->max_update_index)
> @@ -336,7 +336,7 @@ int reftable_writer_add_log(struct refta
>         if (log->value_type == REFTABLE_LOG_DELETION)
>                 return reftable_writer_add_log_verbatim(w, log);
>
> -       if (log->refname == NULL)
> +       if (!log->refname)
>                 return REFTABLE_API_ERROR;
>
>         input_log_message = log->value.update.message;
> @@ -545,7 +545,7 @@ static int writer_finish_public_section(
>         uint8_t typ = 0;
>         int err = 0;
>
> -       if (w->block_writer == NULL)
> +       if (!w->block_writer)
>                 return 0;
>
>         typ = block_writer_type(w->block_writer);
> @@ -694,7 +694,7 @@ static int writer_flush_nonempty_block(s
>
>  static int writer_flush_block(struct reftable_writer *w)
>  {
> -       if (w->block_writer == NULL)
> +       if (!w->block_writer)
>                 return 0;
>         if (w->block_writer->entries == 0)
>                 return 0;
> diff -u -p a/rerere.c b/rerere.c
> --- a/rerere.c
> +++ b/rerere.c
> @@ -591,7 +591,7 @@ int rerere_remaining(struct repository *
>                 else if (conflict_type == RESOLVED) {
>                         struct string_list_item *it;
>                         it = string_list_lookup(merge_rr, (const char *)e->name);
> -                       if (it != NULL) {
> +                       if (it) {
>                                 free_rerere_id(it);
>                                 it->util = RERERE_RESOLVED;
>                         }
> diff -u -p a/revision.c b/revision.c
> --- a/revision.c
> +++ b/revision.c
> @@ -2833,7 +2833,7 @@ int setup_revisions(int argc, const char
>         }
>         strvec_clear(&prune_data);
>
> -       if (revs->def == NULL)
> +       if (!revs->def)
>                 revs->def = opt ? opt->def : NULL;
>         if (opt && opt->tweak)
>                 opt->tweak(revs, opt);
> @@ -3652,7 +3652,7 @@ static enum rewrite_result rewrite_one_1
>                         return rewrite_one_ok;
>                 if (!p->parents)
>                         return rewrite_one_noparents;
> -               if ((p = one_relevant_parent(revs, p->parents)) == NULL)
> +               if (!(p = one_relevant_parent(revs, p->parents)))
>                         return rewrite_one_ok;
>                 *pp = p;
>         }
> diff -u -p a/setup.c b/setup.c
> --- a/setup.c
> +++ b/setup.c
> @@ -1470,7 +1470,7 @@ int git_config_perm(const char *var, con
>         int i;
>         char *endptr;
>
> -       if (value == NULL)
> +       if (!value)
>                 return PERM_GROUP;
>
>         if (!strcmp(value, "umask"))
> diff -u -p a/sh-i18n--envsubst.c b/sh-i18n--envsubst.c
> --- a/sh-i18n--envsubst.c
> +++ b/sh-i18n--envsubst.c
> @@ -397,7 +397,7 @@ subst_from_stdin (void)
>                   /* Substitute the variable's value from the environment.  */
>                   const char *env_value = getenv (buffer);
>
> -                 if (env_value != NULL)
> +                 if (env_value)
>                     fputs (env_value, stdout);
>                 }
>               else
> diff -u -p a/shallow.c b/shallow.c
> --- a/shallow.c
> +++ b/shallow.c
> @@ -560,7 +560,7 @@ static void paint_down(struct paint_info
>                 else
>                         c->object.flags |= SEEN;
>
> -               if (*refs == NULL)
> +               if (!*refs)
>                         *refs = bitmap;
>                 else {
>                         memcpy(tmp, *refs, bitmap_size);
> diff -u -p a/trailer.c b/trailer.c
> --- a/trailer.c
> +++ b/trailer.c
> @@ -1029,7 +1029,7 @@ static FILE *create_in_place_tempfile(co
>
>         /* Create temporary file in the same directory as the original */
>         tail = strrchr(file, '/');
> -       if (tail != NULL)
> +       if (tail)
>                 strbuf_add(&filename_template, file, tail - file + 1);
>         strbuf_addstr(&filename_template, "git-interpret-trailers-XXXXXX");
>
> diff -u -p a/transport.c b/transport.c
> --- a/transport.c
> +++ b/transport.c
> @@ -438,7 +438,7 @@ static int fetch_refs_via_pack(struct tr
>                 args.self_contained_and_connected;
>         data->options.connectivity_checked = args.connectivity_checked;
>
> -       if (refs == NULL)
> +       if (!refs)
>                 ret = -1;
>         if (report_unmatched_refs(to_fetch, nr_heads))
>                 ret = -1;
> diff -u -p a/wildmatch.c b/wildmatch.c
> --- a/wildmatch.c
> +++ b/wildmatch.c
> @@ -113,7 +113,7 @@ static int dowild(const uchar *p, const
>                                 /* Trailing "**" matches everything.  Trailing "*" matches
>                                  * only if there are no more slash characters. */
>                                 if (!match_slash) {
> -                                       if (strchr((char*)text, '/') != NULL)
> +                                       if (strchr((char *)text, '/'))
>                                                 return WM_NOMATCH;
>                                 }
>                                 return WM_MATCH;
> diff -u -p a/worktree.c b/worktree.c
> --- a/worktree.c
> +++ b/worktree.c
> @@ -483,7 +483,7 @@ int submodule_uses_worktrees(const char
>                 return 0;
>
>         d = readdir_skip_dot_and_dotdot(dir);
> -       if (d != NULL)
> +       if (d)
>                 ret = 1;
>         closedir(dir);
>         return ret;
> diff -u -p a/wrapper.c b/wrapper.c
> --- a/wrapper.c
> +++ b/wrapper.c
> @@ -393,7 +393,7 @@ FILE *xfopen(const char *path, const cha
>  FILE *xfdopen(int fd, const char *mode)
>  {
>         FILE *stream = fdopen(fd, mode);
> -       if (stream == NULL)
> +       if (!stream)
>                 die_errno("Out of memory? fdopen failed");
>         return stream;
>  }
> diff -u -p a/xdiff-interface.c b/xdiff-interface.c
> --- a/xdiff-interface.c
> +++ b/xdiff-interface.c
> @@ -159,7 +159,7 @@ int read_mmfile(mmfile_t *ptr, const cha
>
>         if (stat(filename, &st))
>                 return error_errno("Could not stat %s", filename);
> -       if ((f = fopen(filename, "rb")) == NULL)
> +       if (!(f = fopen(filename, "rb")))
>                 return error_errno("Could not open %s", filename);
>         sz = xsize_t(st.st_size);
>         ptr->ptr = xmalloc(sz ? sz : 1);
> diff -u -p a/xdiff/xemit.c b/xdiff/xemit.c
> --- a/xdiff/xemit.c
> +++ b/xdiff/xemit.c
> @@ -65,7 +65,7 @@ xdchange_t *xdl_get_hunk(xdchange_t **xs
>                         *xscr = xch;
>         }
>
> -       if (*xscr == NULL)
> +       if (!*xscr)
>                 return NULL;
>
>         lxch = *xscr;
> diff -u -p a/xdiff/xprepare.c b/xdiff/xprepare.c
> --- a/xdiff/xprepare.c
> +++ b/xdiff/xprepare.c
> @@ -188,7 +188,7 @@ static int xdl_prepare_ctx(unsigned int
>         memset(rhash, 0, hsize * sizeof(xrecord_t *));
>
>         nrec = 0;
> -       if ((cur = blk = xdl_mmfile_first(mf, &bsize)) != NULL) {
> +       if ((cur = blk = xdl_mmfile_first(mf, &bsize))) {
>                 for (top = blk + bsize; cur < top; ) {
>                         prev = cur;
>                         hav = xdl_hash_record(&cur, top, xpp->flags);
> diff -u -p a/xdiff/xutils.c b/xdiff/xutils.c
> --- a/xdiff/xutils.c
> +++ b/xdiff/xutils.c
> @@ -122,7 +122,7 @@ long xdl_guess_lines(mmfile_t *mf, long
>         long nl = 0, size, tsize = 0;
>         char const *data, *cur, *top;
>
> -       if ((cur = data = xdl_mmfile_first(mf, &size)) != NULL) {
> +       if ((cur = data = xdl_mmfile_first(mf, &size))) {
>                 for (top = data + size; nl < sample && cur < top; ) {
>                         nl++;
>                         if (!(cur = memchr(cur, '\n', top - cur)))

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

[Philip Oakley]

On 01/05/2022 18:04, Elia Pinto wrote:
> 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

>> 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]

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

[Junio C Hamano]

Elia Pinto <gitter.spiros@gmail.com> writes:

>> 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

Hmph.  Even if these patches were created by coccicheck we should
sanity check them to make sure we are not applying some stupid and
obvious mistakes, but if they were created by an ad-hoc tool, it
means we would need to check a lot more careful than a patch that
was done with a known tool with a clear rule (which is what running
"make coccicheck" with your new rule file would have given us).

To avoid unnecessary conflicts with in-flight topics, ideally, we
perhaps could do something along this line:

 * Pick a recent stable point that is an ancestor of all topics in
   flight.  Add the new coccinelle rule file, take "make coccicheck"
   output and create a two-patch series like Philip suggested.  Queue
   the result in a topic branch B.

 * For each topic in flight T, make a trial merge of T into B, and
   examine "make coccicheck" output.  Any new breakages such a test
   finds are new violations the topic T introduces.  Discard the
   result of the trial merge, and add one commit to topic T that
   corrects the violations the topic introduced, and send that fixup
   to the author of the topic for consideration when the topic is
   rerolled (or if the topic is in 'next', acked to be queued on
   top).  Do not fix the violations that is corrected when branch B
   was prepared above.

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.

Thanks.

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

[Elia Pinto]

Il giorno lun 2 mag 2022 alle ore 01:14 Junio C Hamano
<gitster@pobox.com> ha scritto:
>
> Elia Pinto <gitter.spiros@gmail.com> writes:
>
> >> 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
>
> Hmph.  Even if these patches were created by coccicheck we should
> sanity check them to make sure we are not applying some stupid and
> obvious mistakes, but if they were created by an ad-hoc tool, it
> means we would need to check a lot more careful than a patch that
> was done with a known tool with a clear rule (which is what running
> "make coccicheck" with your new rule file would have given us).
>
> To avoid unnecessary conflicts with in-flight topics, ideally, we
> perhaps could do something along this line:
>
>  * Pick a recent stable point that is an ancestor of all topics in
>    flight.  Add the new coccinelle rule file, take "make coccicheck"
>    output and create a two-patch series like Philip suggested.  Queue
>    the result in a topic branch B.
>
>  * For each topic in flight T, make a trial merge of T into B, and
>    examine "make coccicheck" output.  Any new breakages such a test
>    finds are new violations the topic T introduces.  Discard the
>    result of the trial merge, and add one commit to topic T that
>    corrects the violations the topic introduced, and send that fixup
>    to the author of the topic for consideration when the topic is
>    rerolled (or if the topic is in 'next', acked to be queued on
>    top).  Do not fix the violations that is corrected when branch B
>    was prepared above.
>
> 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.
>
> Thanks.
Thank you but it seems like a very heavy work for something that had
to be much simpler. IMHO my contribution, if deemed useful, ends with
the patch that adds ONLY the cocci script, which can solve a style
problem identified by the documentation. And it sure works, if
applied. But it is not necessarily the case that there must be to the
same time a patch series that actually applies the script to the git
codebase, in any development branch. This can be done at the most
opportune moment by anyone who wishes it, from my POV.

Again, thanks you all

Best Regards
>
>
>

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

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> To avoid unnecessary conflicts with in-flight topics, ideally, we
> perhaps could do something along this line:
>
>  * Pick a recent stable point that is an ancestor of all topics in
>    flight.  Add the new coccinelle rule file, take "make coccicheck"
>    output and create a two-patch series like Philip suggested.  Queue
>    the result in a topic branch B.
>
>  * For each topic in flight T, make a trial merge of T into B, and
>    examine "make coccicheck" output.  Any new breakages such a test
>    finds are new violations the topic T introduces.  Discard the
>    result of the trial merge, and add one commit to topic T that
>    corrects the violations the topic introduced, and send that fixup
>    to the author of the topic for consideration when the topic is
>    rerolled (or if the topic is in 'next', acked to be queued on
>    top).  Do not fix the violations that is corrected when branch B
>    was prepared above.
>
> 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.

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

[Philip Oakley]

On 02/05/2022 07:22, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> To avoid unnecessary conflicts with in-flight topics, ideally, we
>> perhaps could do something along this line:
>>
>>  * Pick a recent stable point that is an ancestor of all topics in
>>    flight.  Add the new coccinelle rule file, take "make coccicheck"
>>    output and create a two-patch series like Philip suggested.  Queue
>>    the result in a topic branch B.
>>
>>  * For each topic in flight T, make a trial merge of T into B, and
>>    examine "make coccicheck" output.  Any new breakages such a test
>>    finds are new violations the topic T introduces.  Discard the
>>    result of the trial merge, and add one commit to topic T that
>>    corrects the violations the topic introduced, and send that fixup
>>    to the author of the topic for consideration when the topic is
>>    rerolled (or if the topic is in 'next', acked to be queued on
>>    top).  Do not fix the violations that is corrected when branch B
>>    was prepared above.
>>
>> 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..

Philip

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

[Carlo Marcelo Arenas Belón]

On Sat, Apr 30, 2022 at 05:20:03PM -0700, Junio C Hamano wrote:
> diff -u -p a/compat/mkdir.c b/compat/mkdir.c
> --- a/compat/mkdir.c
> +++ b/compat/mkdir.c
> @@ -9,7 +9,7 @@ int compat_mkdir_wo_trailing_slash(const
>  	size_t len = strlen(dir);
>  
>  	if (len && dir[len-1] == '/') {
> -		if ((tmp_dir = strdup(dir)) == NULL)
> +		if (!(tmp_dir = strdup(dir)))

did not check them all, but wanted to raise that this specific case, seems
to be regressing in readability.

sure; it fails another rule of "not doing assignments inside if", but maybe
when it was introduced with 0539ecfdfce (compat: some mkdir() do not like a
slash at the end, 2012-08-24), that wasn't valid or it was considered ok
for code in compat.

but more importantly it serves as an example of why most of those rules
use "try"; after all the assignment WITH the parenthesis and == is clear
enough that might not warrant the need of two lines, but without the ==
it is likely to cause trouble or confuse someone.

Carlo

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

[Junio C Hamano]

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;
 
 	/*

Re: [PATCH 06/23] builtin/bisect--helper.c: Fix coding style

[Christian Couder]

About the commit subject, I think something like the following would be better:

"bisect--helper: fix comparison with NULL"

This way it doesn't use an uppercase for "fix", the area part is
smaller, it's more specific about what the patch is doing.

On Tue, May 3, 2022 at 3:15 AM Elia Pinto <gitter.spiros@gmail.com> wrote:
>
> Adhere to the git coding style which requires "Do not explicitly compute an
> integral value with constant 0 or '\ 0', or a pointer value with constant NULL."

The actual wording is:

"Do not explicitly compare an integral value with constant 0 or '\0',
or a pointer value with constant NULL."

which is a bit different from what you wrote. (It's "compare" not
"compute" and "'\0'" not "'\ 0'".)

The rest looks fine.