Re: [PATCH v4 13/16] builtin/blame: fix type of `length` variable when emitting object ID

> The `length` variable is used to store how many bytes we wish to emit
> from an object ID. This value will either be the full hash algorithm's
> length, or the abbreviated hash that can be set via `--abbrev` or the
> "core.abbrev" option. The former is of type `size_t`, whereas the latter
> is of type `int`, which causes a warning with "-Wsign-compare".
>
> The reason why `abbrev` is using a signed type is mostly that it is
> initialized with `-1` to indicate that we have to compute the minimum
> abbreviation length. This length is computed via `find_alignment()`,
> which always gets called before `emit_other()`, and thus we can assume
> that the value would never be negative in `emit_other()`.
>
> In fact, we can even assume that the value will always be at least
> `MINIMUM_ABBREV`, which is enforced by both `git_default_core_config()`
> and `parse_opt_abbrev_cb()`. We implicitly rely on this by subtracting
> up to 3 without checking for whether the value becomes negative. We then
> pass the value to printf(3p) to print the prefix of our object's ID, so
> if that assumption was violated we may end up with undefined behaviour.
>
> Squelch the warning by asserting this invariant and casting the value of
> `abbrev` to `size_t`. This allows us to store the whole length as an
> unsigned integer, which we can then pass to `fwrite()`.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/blame.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index b33b44c89a431d45e05d9863f69c049ba5eec08c..867032e4c16878ffd56df8a73162b89ca4bd2694 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -6,7 +6,6 @@
>   */
>
>  #define USE_THE_REPOSITORY_VARIABLE
> -#define DISABLE_SIGN_COMPARE_WARNINGS
>
>  #include "builtin.h"
>  #include "config.h"
> @@ -468,9 +467,14 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int
>  		reset = GIT_COLOR_RESET;
>  	}
>
> +	if (abbrev < MINIMUM_ABBREV)
> +		BUG("abbreviation is smaller than minimum length: %d < %d",
> +		    abbrev, MINIMUM_ABBREV);
> +
>  	for (cnt = 0; cnt < ent->num_lines; cnt++) {
>  		char ch;
> -		int length = (opt & OUTPUT_LONG_OBJECT_NAME) ? the_hash_algo->hexsz : abbrev;
> +		size_t length = (opt & OUTPUT_LONG_OBJECT_NAME) ?
> +			the_hash_algo->hexsz : (size_t) abbrev;
>
>  		if (opt & OUTPUT_COLOR_LINE) {
>  			if (cnt > 0) {
> @@ -501,7 +505,7 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int
>  			length--;
>  			putchar('?');
>  		}
> -		printf("%.*s", length, hex);
> +		fwrite(hex, 1, length, stdout);

>  		if (opt & OUTPUT_ANNOTATE_COMPAT) {
>  			const char *name;
>  			if (opt & OUTPUT_SHOW_EMAIL)
>
> --
> 2.47.0.366.g5daf58cba8.dirty
>
>
>