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