diff mbox series

[v2,3/8] object.c: add and use oid_is_type_or_die_msg() function

Message ID patch-3.8-22e7d9a3db-20210420T133218Z-avarab@gmail.com (mailing list archive)
State New
Headers show
Series object.c: add and use "is expected" utility function + object_as_type() use | expand

Commit Message

Ævar Arnfjörð Bjarmason April 20, 2021, 1:36 p.m. UTC
Add a oid_is_type_or_die_msg() function to go with the "error" and
"die" forms for emitting "expected type X, got Y" messages. This is
useful for callers that want the message itself as a char *.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 merge-recursive.c |  6 ++++--
 object.c          | 12 ++++++++++++
 object.h          |  3 +++
 3 files changed, 19 insertions(+), 2 deletions(-)

Comments

Jonathan Tan April 21, 2021, 10:07 p.m. UTC | #1
> +char* oid_is_type_or_die_msg(const struct object_id *oid,
> +				   enum object_type want,
> +				   enum object_type *type)
> +{
> +	struct strbuf sb = STRBUF_INIT;
> +	if (want == *type)
> +		BUG("call this just to get the message!");
> +	strbuf_addf(&sb, _(object_type_mismatch_msg), oid_to_hex(oid),
> +		    type_name(*type), type_name(want));
> +	return strbuf_detach(&sb, NULL);
> +}

It would be more convenient for the caller if this function also checks
want vs type and returns NULL if they match. That would also be more
consistent with the other functions, and the caller wouldn't need to
remember that this function only works if want and type are different.
Josh Steadmon April 21, 2021, 11:28 p.m. UTC | #2
On 2021.04.20 15:36, Ævar Arnfjörð Bjarmason wrote:
> diff --git a/object.c b/object.c
> index 9e06c0ee92..0f07f976fb 100644
> --- a/object.c
> +++ b/object.c
> @@ -176,6 +176,18 @@ int oid_is_type_or_error(const struct object_id *oid,
>  		     type_name(want));
>  }
>  
> +char* oid_is_type_or_die_msg(const struct object_id *oid,

It's kind of a nitpick, but I found the function name to be confusing.
It sounds like you're going to die with a custom message. Maybe
something like "get_oid_type_mismatch_msg()" would be more
straightforward.
Junio C Hamano April 28, 2021, 4:12 a.m. UTC | #3
Josh Steadmon <steadmon@google.com> writes:

> On 2021.04.20 15:36, Ævar Arnfjörð Bjarmason wrote:
>> diff --git a/object.c b/object.c
>> index 9e06c0ee92..0f07f976fb 100644
>> --- a/object.c
>> +++ b/object.c
>> @@ -176,6 +176,18 @@ int oid_is_type_or_error(const struct object_id *oid,
>>  		     type_name(want));
>>  }
>>  
>> +char* oid_is_type_or_die_msg(const struct object_id *oid,
>
> It's kind of a nitpick, but I found the function name to be confusing.
> It sounds like you're going to die with a custom message. Maybe
> something like "get_oid_type_mismatch_msg()" would be more
> straightforward.

Yeah, in an older round I found this function's name was confusing,
too.

Also, there is a style (in our codebase, asterisk to signal the
pointer-ness sticks to the identifier, not to the type name).

Thanks.
diff mbox series

Patch

diff --git a/merge-recursive.c b/merge-recursive.c
index b952106203..c74239544f 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -2999,9 +2999,11 @@  static int read_oid_strbuf(struct merge_options *opt,
 	if (!buf)
 		return err(opt, _("cannot read object %s"), oid_to_hex(oid));
 	if (type != OBJ_BLOB) {
-		const char* msg = oid_is_type_or_die_msg(oid, OBJ_BLOB, &type);
+		char *msg = oid_is_type_or_die_msg(oid, OBJ_BLOB, &type);
+		int ret = err(opt, msg);
 		free(buf);
-		return err(opt, _("object %s is not a blob"), oid_to_hex(oid));
+		free(msg);
+		return ret;
 	}
 	strbuf_attach(dst, buf, size, size + 1);
 	return 0;
diff --git a/object.c b/object.c
index 9e06c0ee92..0f07f976fb 100644
--- a/object.c
+++ b/object.c
@@ -176,6 +176,18 @@  int oid_is_type_or_error(const struct object_id *oid,
 		     type_name(want));
 }
 
+char* oid_is_type_or_die_msg(const struct object_id *oid,
+				   enum object_type want,
+				   enum object_type *type)
+{
+	struct strbuf sb = STRBUF_INIT;
+	if (want == *type)
+		BUG("call this just to get the message!");
+	strbuf_addf(&sb, _(object_type_mismatch_msg), oid_to_hex(oid),
+		    type_name(*type), type_name(want));
+	return strbuf_detach(&sb, NULL);
+}
+
 void *object_as_type(struct object *obj, enum object_type type, int quiet)
 {
 	if (obj->type == type)
diff --git a/object.h b/object.h
index f8609a8518..7ae6407598 100644
--- a/object.h
+++ b/object.h
@@ -127,6 +127,9 @@  void oid_is_type_or_die(const struct object_id *oid, enum object_type want,
 			enum object_type *type);
 int oid_is_type_or_error(const struct object_id *oid, enum object_type want,
 			 enum object_type *type);
+char* oid_is_type_or_die_msg(const struct object_id *oid,
+			     enum object_type want,
+			     enum object_type *type);
 
 /*
  * Returns the object, having parsed it to find out what it is.