Subject: Re: [PATCH v3 2/2] column: guard against negative padding

From: Junio C Hamano <[email-masked]>
Date: Tue, 13 Feb 2024 11:39:11 -0800

Rubén Justo <[email-masked]> writes:

>> This one happens to be safe currently because "git tag" passes 2 in
>> opts->padding, but I do not think this is needed.
>
> At first glance, I also thought this was not necessary.
>
> However, callers of run_column_filter() might forget to check the return
> value, and the BUG() triggered by the underlying process could be buried
> and ignored.  Having the BUG() here, in the same process, makes it more
> noticeable.

The point of BUG() is to help developers catch the silly breakage
before it excapes from the lab, and we can expect these careless
developers to ignore the return value.  But "column --padding=-1"
invoked as a subprocess will show a human-readable error message
to such a developer, so it is less important than the BUG() in the
other place.

There is no black or white decision, but this one is much less
darker gray than the other one is.