Re: [RFC PATCH] revert: Use assert to catch inherent program bugs

From: Junio C Hamano <gitster@pobox.com>
Date: May 20, 2011, 17:03 UTC
Message-ID: <7v7h9l4855.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1305880223-7542-2-git-send-email-artagnon@gmail.com>

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Instead of returning and error status or calling die, use an assert
> statement to guard against callers who don't call the functions with
> sane arguments.

Please do not do this. assert() is a mechanism to aid debugging in a development build, and can be stripped away in the production build.

Show 12 quoted lines

> diff --git a/builtin/revert.c b/builtin/revert.c
> index f697e66..8102d77 100644
> --- a/builtin/revert.c
> +++ b/builtin/revert.c
> @@ -123,8 +123,7 @@ static int get_message(const char *raw_message, struct commit_message *out)
>  	int abbrev_len, subject_len;
>  	char *q;
>  
> -	if (!raw_message)
> -		return -1;
> +	assert(raw_message);
>  	encoding = get_encoding(raw_message);

This change is wrong, as you could feed a NULL to get_encoding (and you also broke that function in the later hunk).

If the calling convention of this internal function is that passing NULL is a programming bug, please do something like this instead:

	if (!raw_message)
		die("BUG: get_message() called with NULL");