git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 3/3] contrib/coccinelle: convert all conditional bugs to bug_on

From
Stefan Beller <sbeller@google.com>
Date
Nov 22, 2017, 22:38 UTC
Message-ID
<20171122223827.26773-4-sbeller@google.com>
In-Reply-To
<20171122223827.26773-1-sbeller@google.com>

Add a coccinelle patch that detects a conditional BUG and converts it to BUG_ON. Also apply that semantic patch.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/merge.c                 |  3 +--
 contrib/coccinelle/bug_on.cocci |  8 ++++++++
 environment.c                   | 22 ++++++++++------------
 notes.c                         |  9 +++++----
 refs.c                          |  7 +++----
 refs/files-backend.c            | 14 ++++++--------
 refs/packed-backend.c           | 13 +++++--------
 sha1_file.c                     |  4 ++--
 tempfile.c                      | 34 ++++++++++++++++------------------
 9 files changed, 56 insertions(+), 58 deletions(-)
 create mode 100644 contrib/coccinelle/bug_on.cocci
diff --git a/builtin/merge.c b/builtin/merge.c
index 612dd7bfb6..df5884b4c1 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -770,8 +770,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 	struct strbuf msg = STRBUF_INIT;
 	strbuf_addbuf(&msg, &merge_msg);
 	strbuf_addch(&msg, '\n');
-	if (squash)
-		BUG("the control must not reach here under --squash");
+	BUG_ON(squash, "the control must not reach here under --squash");
 	if (0 < option_edit)
 		strbuf_commented_addf(&msg, _(merge_editor_comment), comment_line_char);
 	if (signoff)
diff --git a/contrib/coccinelle/bug_on.cocci b/contrib/coccinelle/bug_on.cocci
new file mode 100644
index 0000000000..e778d19e3c
--- /dev/null
+++ b/contrib/coccinelle/bug_on.cocci
@@ -0,0 +1,8 @@
+@@
+expression E;
+@@
+- if (E)
+-  BUG(
++ BUG_ON(E,
+   ...);
+
diff --git a/environment.c b/environment.c
index 8fa032f307..9ba01a85ec 100644
--- a/environment.c
+++ b/environment.c
@@ -177,22 +177,20 @@ int have_git_dir(void)
 
 const char *get_git_dir(void)
 {
-	if (!the_repository->gitdir)
-		BUG("git environment hasn't been setup");
+	BUG_ON(!the_repository->gitdir, "git environment hasn't been setup");
 	return the_repository->gitdir;
 }
 
 const char *get_git_common_dir(void)
 {
-	if (!the_repository->commondir)
-		BUG("git environment hasn't been setup");
+	BUG_ON(!the_repository->commondir,
+	       "git environment hasn't been setup");
 	return the_repository->commondir;
 }
 
 const char *get_git_namespace(void)
 {
-	if (!namespace)
-		BUG("git environment hasn't been setup");
+	BUG_ON(!namespace, "git environment hasn't been setup");
 	return namespace;
 }
 
@@ -242,8 +240,8 @@ const char *get_git_work_tree(void)
 
 char *get_object_directory(void)
 {
-	if (!the_repository->objectdir)
-		BUG("git environment hasn't been setup");
+	BUG_ON(!the_repository->objectdir,
+	       "git environment hasn't been setup");
 	return the_repository->objectdir;
 }
 
@@ -282,15 +280,15 @@ int odb_pack_keep(const char *name)
 
 char *get_index_file(void)
 {
-	if (!the_repository->index_file)
-		BUG("git environment hasn't been setup");
+	BUG_ON(!the_repository->index_file,
+	       "git environment hasn't been setup");
 	return the_repository->index_file;
 }
 
 char *get_graft_file(void)
 {
-	if (!the_repository->graft_file)
-		BUG("git environment hasn't been setup");
+	BUG_ON(!the_repository->graft_file,
+	       "git environment hasn't been setup");
 	return the_repository->graft_file;
 }
 
diff --git a/notes.c b/notes.c
index c7f21fae44..cae8fa0657 100644
--- a/notes.c
+++ b/notes.c
@@ -400,10 +400,11 @@ static void load_subtree(struct notes_tree *t, struct leaf_node *subtree,
 		     oid_to_hex(&subtree->val_oid));
 
 	prefix_len = subtree->key_oid.hash[KEY_INDEX];
-	if (prefix_len >= GIT_SHA1_RAWSZ)
-		BUG("prefix_len (%"PRIuMAX") is out of range", (uintmax_t)prefix_len);
-	if (prefix_len * 2 < n)
-		BUG("prefix_len (%"PRIuMAX") is too small", (uintmax_t)prefix_len);
+	BUG_ON(prefix_len >= GIT_SHA1_RAWSZ,
+	       "prefix_len (%"PRIuMAX") is out of range",
+	       (uintmax_t)prefix_len);
+	BUG_ON(prefix_len * 2 < n, "prefix_len (%"PRIuMAX") is too small",
+	       (uintmax_t)prefix_len);
 	memcpy(object_oid.hash, subtree->key_oid.hash, prefix_len);
 	while (tree_entry(&desc, &entry)) {
 		unsigned char type;
diff --git a/refs.c b/refs.c
index 339d4318ee..1a5473fbe3 100644
--- a/refs.c
+++ b/refs.c
@@ -937,8 +937,8 @@ int ref_transaction_update(struct ref_transaction *transaction,
 		return -1;
 	}
 
-	if (flags & ~REF_TRANSACTION_UPDATE_ALLOWED_FLAGS)
-		BUG("illegal flags 0x%x passed to ref_transaction_update()", flags);
+	BUG_ON(flags & ~REF_TRANSACTION_UPDATE_ALLOWED_FLAGS,
+	       "illegal flags 0x%x passed to ref_transaction_update()", flags);
 
 	flags |= (new_oid ? REF_HAVE_NEW : 0) | (old_oid ? REF_HAVE_OLD : 0);
 
@@ -1279,8 +1279,7 @@ struct ref_iterator *refs_ref_iterator_begin(
 		iter = prefix_ref_iterator_begin(iter, "", trim);
 
 	/* Sanity check for subclasses: */
-	if (!iter->ordered)
-		BUG("reference iterator is not ordered");
+	BUG_ON(!iter->ordered, "reference iterator is not ordered");
 
 	return iter;
 }
diff --git a/refs/files-backend.c b/refs/files-backend.c
index f75d960e19..a00a7d9f78 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2208,8 +2208,8 @@ static int split_head_update(struct ref_update *update,
 	 * size, but it happens at most once per transaction.
 	 * Add new_update->refname instead of a literal "HEAD".
 	 */
-	if (strcmp(new_update->refname, "HEAD"))
-		BUG("%s unexpectedly not 'HEAD'", new_update->refname);
+	BUG_ON(strcmp(new_update->refname, "HEAD"),
+	       "%s unexpectedly not 'HEAD'", new_update->refname);
 	item = string_list_insert(affected_refnames, new_update->refname);
 	item->util = new_update;
 
@@ -2285,9 +2285,8 @@ static int split_symref_update(struct files_ref_store *refs,
 	 * referent, which might soon be freed by our caller.
 	 */
 	item = string_list_insert(affected_refnames, new_update->refname);
-	if (item->util)
-		BUG("%s unexpectedly found in affected_refnames",
-		    new_update->refname);
+	BUG_ON(item->util, "%s unexpectedly found in affected_refnames",
+	       new_update->refname);
 	item->util = new_update;
 
 	return 0;
@@ -2570,9 +2569,8 @@ static int files_transaction_prepare(struct ref_store *ref_store,
 		struct string_list_item *item =
 			string_list_append(&affected_refnames, update->refname);
 
-		if ((update->flags & REF_IS_PRUNING) &&
-		    !(update->flags & REF_NO_DEREF))
-			BUG("REF_IS_PRUNING set without REF_NO_DEREF");
+		BUG_ON((update->flags & REF_IS_PRUNING) && !(update->flags & REF_NO_DEREF),
+		       "REF_IS_PRUNING set without REF_NO_DEREF");
 
 		/*
 		 * We store a pointer to update in item->util, but at
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 023243fd5f..e10c2f94d8 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -334,9 +334,7 @@ static void sort_snapshot(struct snapshot *snapshot)
 
 	while (pos < eof) {
 		eol = memchr(pos, '\n', eof - pos);
-		if (!eol)
-			/* The safety check should prevent this. */
-			BUG("unterminated line found in packed-refs");
+		BUG_ON(!eol, "unterminated line found in packed-refs");
 		if (eol - pos < GIT_SHA1_HEXSZ + 2)
 			die_invalid_line(snapshot->refs->path,
 					 pos, eof - pos);
@@ -349,9 +347,8 @@ static void sort_snapshot(struct snapshot *snapshot)
 			const char *peeled_start = eol;
 
 			eol = memchr(peeled_start, '\n', eof - peeled_start);
-			if (!eol)
-				/* The safety check should prevent this. */
-				BUG("unterminated peeled line found in packed-refs");
+			BUG_ON(!eol,
+			       "unterminated peeled line found in packed-refs");
 			eol++;
 		}
 
@@ -1272,8 +1269,8 @@ int is_packed_transaction_needed(struct ref_store *ref_store,
 	size_t i;
 	int ret;
 
-	if (!is_lock_file_locked(&refs->lock))
-		BUG("is_packed_transaction_needed() called while unlocked");
+	BUG_ON(!is_lock_file_locked(&refs->lock),
+	       "is_packed_transaction_needed() called while unlocked");
 
 	/*
 	 * We're only going to bother returning false for the common,
diff --git a/sha1_file.c b/sha1_file.c
index 8a7c6b7eba..f10886f907 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1885,8 +1885,8 @@ int for_each_file_in_obj_subdir(unsigned int subdir_nr,
 	int r = 0;
 	struct object_id oid;
 
-	if (subdir_nr > 0xff)
-		BUG("invalid loose object subdirectory: %x", subdir_nr);
+	BUG_ON(subdir_nr > 0xff, "invalid loose object subdirectory: %x",
+	       subdir_nr);
 
 	origlen = path->len;
 	strbuf_complete(path, '/');
diff --git a/tempfile.c b/tempfile.c
index 5fdafdd2d2..63884b6bdd 100644
--- a/tempfile.c
+++ b/tempfile.c
@@ -107,8 +107,8 @@ static void activate_tempfile(struct tempfile *tempfile)
 {
 	static int initialized;
 
-	if (is_tempfile_active(tempfile))
-		BUG("activate_tempfile called for active object");
+	BUG_ON(is_tempfile_active(tempfile),
+	       "activate_tempfile called for active object");
 
 	if (!initialized) {
 		sigchain_push_common(remove_tempfiles_on_signal);
@@ -215,10 +215,9 @@ struct tempfile *xmks_tempfile_m(const char *template, int mode)
 
 FILE *fdopen_tempfile(struct tempfile *tempfile, const char *mode)
 {
-	if (!is_tempfile_active(tempfile))
-		BUG("fdopen_tempfile() called for inactive object");
-	if (tempfile->fp)
-		BUG("fdopen_tempfile() called for open object");
+	BUG_ON(!is_tempfile_active(tempfile),
+	       "fdopen_tempfile() called for inactive object");
+	BUG_ON(tempfile->fp, "fdopen_tempfile() called for open object");
 
 	tempfile->fp = fdopen(tempfile->fd, mode);
 	return tempfile->fp;
@@ -226,22 +225,22 @@ FILE *fdopen_tempfile(struct tempfile *tempfile, const char *mode)
 
 const char *get_tempfile_path(struct tempfile *tempfile)
 {
-	if (!is_tempfile_active(tempfile))
-		BUG("get_tempfile_path() called for inactive object");
+	BUG_ON(!is_tempfile_active(tempfile),
+	       "get_tempfile_path() called for inactive object");
 	return tempfile->filename.buf;
 }
 
 int get_tempfile_fd(struct tempfile *tempfile)
 {
-	if (!is_tempfile_active(tempfile))
-		BUG("get_tempfile_fd() called for inactive object");
+	BUG_ON(!is_tempfile_active(tempfile),
+	       "get_tempfile_fd() called for inactive object");
 	return tempfile->fd;
 }
 
 FILE *get_tempfile_fp(struct tempfile *tempfile)
 {
-	if (!is_tempfile_active(tempfile))
-		BUG("get_tempfile_fp() called for inactive object");
+	BUG_ON(!is_tempfile_active(tempfile),
+	       "get_tempfile_fp() called for inactive object");
 	return tempfile->fp;
 }
 
@@ -275,10 +274,9 @@ int close_tempfile_gently(struct tempfile *tempfile)
 
 int reopen_tempfile(struct tempfile *tempfile)
 {
-	if (!is_tempfile_active(tempfile))
-		BUG("reopen_tempfile called for an inactive object");
-	if (0 <= tempfile->fd)
-		BUG("reopen_tempfile called for an open object");
+	BUG_ON(!is_tempfile_active(tempfile),
+	       "reopen_tempfile called for an inactive object");
+	BUG_ON(0 <= tempfile->fd, "reopen_tempfile called for an open object");
 	tempfile->fd = open(tempfile->filename.buf, O_WRONLY);
 	return tempfile->fd;
 }
@@ -287,8 +285,8 @@ int rename_tempfile(struct tempfile **tempfile_p, const char *path)
 {
 	struct tempfile *tempfile = *tempfile_p;
 
-	if (!is_tempfile_active(tempfile))
-		BUG("rename_tempfile called for inactive object");
+	BUG_ON(!is_tempfile_active(tempfile),
+	       "rename_tempfile called for inactive object");
 
 	if (close_tempfile_gently(tempfile)) {
 		delete_tempfile(tempfile_p);
-- 
2.15.0.448.gf294e3d99a-goog
Previous: Jeff KingNext: Stefan Beller
Message 5 of 18 in “Introduce BUG_ON(cond, msg) MACRO”
  1. 0/3 Introduce BUG_ON(cond, msg) MACROStefan Beller, Nov 22, 2017
  2. 2/3 git-compat: introduce BUG_ON(condition, fmt, ...) macroStefan Beller, Nov 22, 2017
  3. Jonathan NiederNov 22, 2017
  4. Jeff KingNov 22, 2017
  5. 3/3 contrib/coccinelle: convert all conditional bugs to bug_onStefan Beller, Nov 22, 2017
  6. 1/3 Documentation/CodingGuidelines: explain why assert is badStefan Beller, Nov 22, 2017
  7. Jonathan NiederNov 22, 2017
  8. Stefan BellerNov 22, 2017
  9. Jonathan NiederNov 22, 2017
  10. Jeff KingNov 22, 2017
  11. Jonathan NiederNov 22, 2017
  12. Jeff KingNov 22, 2017
  13. Jonathan NiederNov 22, 2017
  14. Jeff KingNov 22, 2017
  15. Jonathan NiederNov 23, 2017
  16. Jeff KingNov 23, 2017
  17. Junio C HamanoNov 23, 2017
  18. Jeff KingNov 23, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.