threads / patch / 57038

patch, 2 partsreplace die("BUG: ...") with BUG("...")

Subject: [PATCH 0/2] replace die("BUG: ...") with BUG("...")

## tl;dr

11 messages between Dec 6, 2021 and Dec 7, 2021. Diffs are folded; open one to read it.

replies: 10people: 2as markdown or json

Ævar Arnfjörð Bjarmason· Dec 6, 2021, 16:25 UTC · lore

A trivial clean-up to change a couple of die() uses to BUG() where appropriate. This is split off from an earlier RFC series I sent in[1], as the range-diff to the relevant patches there shown there are no changes since then.

1. https://lore.kernel.org/git/RFC-patch-07.21-3f897bf6b0e-20211115T220831Z-avarab@gmail.com/

Ævar Arnfjörð Bjarmason (2):
  pack-objects: use BUG(...) not die("BUG: ...")
  strbuf.h: use BUG(...) not die("BUG: ...")

 builtin/pack-objects.c | 2 +-
 strbuf.h               | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Range-diff: 1: 2507ea71700 = 1: 2a17ed9f135 pack-objects: use BUG(...) not die("BUG: ...") 2: 5dedcee3fb0 = 2: ab89fec50c3 strbuf.h: use BUG(...) not die("BUG: ...")

-- 
2.34.1.898.g5a552c2e5f0

Ævar Arnfjörð Bjarmason· Dec 6, 2021, 16:25 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH 1/2] pack-objects: use BUG(...) not die("BUG: ...")

Change this code added in da93d12b004 (pack-objects: be incredibly anal about stdio semantics, 2006-04-02) to use BUG() instead.

See 1a07e59c3e2 (Update messages in preparation for i18n, 2018-07-21) for when the "BUG: " prefix was added, and [1] for background on the Solaris behavior that prompted the exhaustive error checking in this fgets() loop.

1. https://lore.kernel.org/git/824.1144007555@lotus.CS.Berkeley.EDU/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/pack-objects.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to builtin/pack-objects.c +1 −1

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 857be7826f3..b36ed828d8d 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3397,7 +3397,7 @@ static void read_object_list_from_stdin(void)
 			if (feof(stdin))
 				break;
 			if (!ferror(stdin))
-				die("BUG: fgets returned NULL, not EOF, not error!");
+				BUG("fgets returned NULL, not EOF, not error!");
 			if (errno != EINTR)
 				die_errno("fgets");
 			clearerr(stdin);

-- 
2.34.1.898.g5a552c2e5f0

Ævar Arnfjörð Bjarmason· Dec 6, 2021, 16:25 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH 2/2] strbuf.h: use BUG(...) not die("BUG: ...")

In 7141efab248 (strbuf: clarify assertion in strbuf_setlen(), 2011-04-27) this 'die("BUG: "' invocation was added with the rationale that strbuf.c had existing users doing the same, but those users were later changed to use BUG() in 033abf97fcb (Replace all die("BUG: ...") calls by BUG() ones, 2018-05-02). Let's do the same here.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 strbuf.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to strbuf.h +1 −1

diff --git a/strbuf.h b/strbuf.h
index 96512f85b31..76965a17d44 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -160,7 +160,7 @@ void strbuf_grow(struct strbuf *sb, size_t amount);
 static inline void strbuf_setlen(struct strbuf *sb, size_t len)
 {
 	if (len > (sb->alloc ? sb->alloc - 1 : 0))
-		die("BUG: strbuf_setlen() beyond buffer");
+		BUG("strbuf_setlen() beyond buffer");
 	sb->len = len;
 	if (sb->buf != strbuf_slopbuf)
 		sb->buf[len] = '\0';

-- 
2.34.1.898.g5a552c2e5f0

Junio C Hamano· Dec 6, 2021, 18:44 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH 0/2] replace die("BUG: ...") with BUG("...")

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> A trivial clean-up to change a couple of die() uses to BUG() where
> appropriate.

Makes sense. I wonder contrib/coccinelle/ can help us maintain them, though.

Will queue.

Thanks.

Ævar Arnfjörð Bjarmason· Dec 7, 2021, 11:00 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/2] replace die("BUG: ...") with BUG("...")

On Mon, Dec 06 2021, Junio C Hamano wrote:

Show 7 quoted lines

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> A trivial clean-up to change a couple of die() uses to BUG() where
>> appropriate.
>
> Makes sense.  I wonder contrib/coccinelle/ can help us maintain
> them, though.

I tried to come up with a rule for this after browsing around in linux's sources & coccicheck's own test cases, but didn't find any example I could adapt. It's rather thin on string munging like that, and I don't know spatch well enough.

Junio C Hamano· Dec 7, 2021, 00:24 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH 3/2] replace die("BUG: ...") with BUG("...")

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

Show 18 quoted lines

> A trivial clean-up to change a couple of die() uses to BUG() where
> appropriate. This is split off from an earlier RFC series I sent
> in[1], as the range-diff to the relevant patches there shown there are
> no changes since then.
>
> 1. https://lore.kernel.org/git/RFC-patch-07.21-3f897bf6b0e-20211115T220831Z-avarab@gmail.com/
>
> Ævar Arnfjörð Bjarmason (2):
>   pack-objects: use BUG(...) not die("BUG: ...")
>   strbuf.h: use BUG(...) not die("BUG: ...")
>
>  builtin/pack-objects.c | 2 +-
>  strbuf.h               | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> Range-diff:
> 1:  2507ea71700 = 1:  2a17ed9f135 pack-objects: use BUG(...) not die("BUG: ...")
> 2:  5dedcee3fb0 = 2:  ab89fec50c3 strbuf.h: use BUG(...) not die("BUG: ...")

There are a bit more that you didn't include and I am having a hard time guessing why.

 object.c    | 2 +-
 pathspec.h  | 3 +--
 tree-diff.c | 3 +--
 3 files changed, 3 insertions(+), 5 deletions(-)

Show changes to diff +3 −5

diff --git c/object.c w/object.c
index 23a24e678a..4be82c1e7b 100644
--- c/object.c
+++ w/object.c
@@ -199,7 +199,7 @@ struct object *lookup_object_by_type(struct repository *r,
 	case OBJ_BLOB:
 		return (struct object *)lookup_blob(r, oid);
 	default:
-		die("BUG: unknown object type %d", type);
+		BUG("unknown object type %d", type);
 	}
 }
 
diff --git c/pathspec.h w/pathspec.h
index 2341dc9901..402ebb8080 100644
--- c/pathspec.h
+++ w/pathspec.h
@@ -58,8 +58,7 @@ struct pathspec {
 #define GUARD_PATHSPEC(ps, mask) \
 	do { \
 		if ((ps)->magic & ~(mask))	       \
-			die("BUG:%s:%d: unsupported magic %x",	\
-			    __FILE__, __LINE__, (ps)->magic & ~(mask)); \
+			BUG("unsupported magic %x", (ps)->magic & ~(mask)); \
 	} while (0)
 
 /* parse_pathspec flags */
diff --git c/tree-diff.c w/tree-diff.c
index 437c98a70e..69031d7cba 100644
--- c/tree-diff.c
+++ w/tree-diff.c
@@ -603,8 +603,7 @@ static void try_to_follow_renames(const struct object_id *old_oid,
 	 * about dry-run mode and returns wildcard info.
 	 */
 	if (opt->pathspec.has_wildcard)
-		die("BUG:%s:%d: wildcards are not supported",
-		    __FILE__, __LINE__);
+		BUG("wildcards are not supported");
 #endif
 
 	/* Remove the file creation entry from the diff queue, and remember it */

Ævar Arnfjörð Bjarmason· Dec 7, 2021, 11:05 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v2 0/4] replace die("BUG: ...") with BUG("...")

A re-roll of this trivial clean-up series, now with 2x more patches for missed cases pointed-out by Junio at https://lore.kernel.org/git/xmqqsfv5b6u6.fsf@gitster.g/

Ævar Arnfjörð Bjarmason (4):
  pack-objects: use BUG(...) not die("BUG: ...")
  strbuf.h: use BUG(...) not die("BUG: ...")
  pathspec: use BUG(...) not die("BUG:%s:%d....", <file>, <line>)
  object.h: use BUG(...) no die("BUG: ...") in lookup_object_by_type()

 builtin/pack-objects.c | 2 +-
 object.c               | 2 +-
 pathspec.h             | 3 +--
 strbuf.h               | 2 +-
 tree-diff.c            | 3 +--
 5 files changed, 5 insertions(+), 7 deletions(-)

Range-diff against v1: 1: 2a17ed9f135 = 1: 4f39177a763 pack-objects: use BUG(...) not die("BUG: ...") 2: ab89fec50c3 = 2: 6740c5d0da8 strbuf.h: use BUG(...) not die("BUG: ...") -: ----------- > 3: 81e354fa3be pathspec: use BUG(...) not die("BUG:%s:%d....", <file>, <line>) -: ----------- > 4: aaf952a9ede object.h: use BUG(...) no die("BUG: ...") in lookup_object_by_type()

-- 
2.34.1.898.g5a552c2e5f0

Ævar Arnfjörð Bjarmason· Dec 7, 2021, 11:05 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v2 1/4] pack-objects: use BUG(...) not die("BUG: ...")

Change this code added in da93d12b004 (pack-objects: be incredibly anal about stdio semantics, 2006-04-02) to use BUG() instead.

1. https://lore.kernel.org/git/824.1144007555@lotus.CS.Berkeley.EDU/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/pack-objects.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to builtin/pack-objects.c +1 −1

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 857be7826f3..b36ed828d8d 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3397,7 +3397,7 @@ static void read_object_list_from_stdin(void)
 			if (feof(stdin))
 				break;
 			if (!ferror(stdin))
-				die("BUG: fgets returned NULL, not EOF, not error!");
+				BUG("fgets returned NULL, not EOF, not error!");
 			if (errno != EINTR)
 				die_errno("fgets");
 			clearerr(stdin);

-- 
2.34.1.898.g5a552c2e5f0

Ævar Arnfjörð Bjarmason· Dec 7, 2021, 11:05 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v2 2/4] strbuf.h: use BUG(...) not die("BUG: ...")

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 strbuf.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to strbuf.h +1 −1

diff --git a/strbuf.h b/strbuf.h
index 96512f85b31..76965a17d44 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -160,7 +160,7 @@ void strbuf_grow(struct strbuf *sb, size_t amount);
 static inline void strbuf_setlen(struct strbuf *sb, size_t len)
 {
 	if (len > (sb->alloc ? sb->alloc - 1 : 0))
-		die("BUG: strbuf_setlen() beyond buffer");
+		BUG("strbuf_setlen() beyond buffer");
 	sb->len = len;
 	if (sb->buf != strbuf_slopbuf)
 		sb->buf[len] = '\0';

-- 
2.34.1.898.g5a552c2e5f0

Ævar Arnfjörð Bjarmason· Dec 7, 2021, 11:05 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v2 3/4] pathspec: use BUG(...) not die("BUG:%s:%d....", <file>, <line>)

Change code that was added in 8f4f8f4579f (guard against new pathspec magic in pathspec matching code, 2013-07-14) to use the BUG() macro instead of emitting a "fatal" message with the "__FILE__"-name and "__LINE__"-numbers.

The original code predated the existence of the BUG() function, which was added in d8193743e08 (usage.c: add BUG() function, 2017-05-12).

Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 pathspec.h  | 3 +--
 tree-diff.c | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

Show changes to 2 files +2 −4

pathspec.h, tree-diff.c

diff --git a/pathspec.h b/pathspec.h
index 2341dc99010..402ebb80808 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -58,8 +58,7 @@ struct pathspec {
 #define GUARD_PATHSPEC(ps, mask) \
 	do { \
 		if ((ps)->magic & ~(mask))	       \
-			die("BUG:%s:%d: unsupported magic %x",	\
-			    __FILE__, __LINE__, (ps)->magic & ~(mask)); \
+			BUG("unsupported magic %x", (ps)->magic & ~(mask)); \
 	} while (0)
 
 /* parse_pathspec flags */
diff --git a/tree-diff.c b/tree-diff.c
index 437c98a70e4..69031d7cbae 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -603,8 +603,7 @@ static void try_to_follow_renames(const struct object_id *old_oid,
 	 * about dry-run mode and returns wildcard info.
 	 */
 	if (opt->pathspec.has_wildcard)
-		die("BUG:%s:%d: wildcards are not supported",
-		    __FILE__, __LINE__);
+		BUG("wildcards are not supported");
 #endif
 
 	/* Remove the file creation entry from the diff queue, and remember it */

-- 
2.34.1.898.g5a552c2e5f0

Ævar Arnfjörð Bjarmason· Dec 7, 2021, 11:05 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v2 4/4] object.h: use BUG(...) no die("BUG: ...") in lookup_object_by_type()

Adjust code added in 7463064b280 (object.h: add lookup_object_by_type() function, 2021-06-22) to use the BUG() function.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 object.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to object.c +1 −1

diff --git a/object.c b/object.c
index 23a24e678a8..4be82c1e7bc 100644
--- a/object.c
+++ b/object.c
@@ -199,7 +199,7 @@ struct object *lookup_object_by_type(struct repository *r,
 	case OBJ_BLOB:
 		return (struct object *)lookup_blob(r, oid);
 	default:
-		die("BUG: unknown object type %d", type);
+		BUG("unknown object type %d", type);
 	}
 }

-- 
2.34.1.898.g5a552c2e5f0