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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 13, 2024, 19:39 UTC
Message-ID: <xmqqle7o5f34.fsf@gitster.g>
In-Reply-To: <69f60c3a-ff47-4cb9-a229-6c5a36e7d9fa@gmail.com>

Rubén Justo <rjusto@gmail.com> writes:

Show 9 quoted lines

>> 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.