Re: [PATCH 2/2] bisect: rewrite `check_term_format` shell function in C
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- May 4, 2016, 17:49 UTC
- Message-ID
- <CAPig+cTXj2=iV0RnZU2Lpr3aqoqWOhPKNdpmOjdh+GocNHiP6g@mail.gmail.com>
- In-Reply-To
- <CAFZEwPNdmJdt=4Hg7KpfGS51sDeKyAPWgApESf564kt_eaP3Rg@mail.gmail.com>
On Wed, May 4, 2016 at 7:58 AM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
Show 22 quoted lines
> On Wed, May 4, 2016 at 1:58 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> On Wed, May 4, 2016 at 3:36 AM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
>>> On Wed, May 4, 2016 at 12:22 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>>>> On Wed, May 4, 2016 at 1:07 AM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
>>>>> + if (check_refname_format(new_term.buf, flag))
>>>>> + die(_("'%s' is not a valid term\n"), term);
>>>>
>>>> Why does this die() while the other "invalid" cases merely return
>>>> error()? What makes this special?
>>>
>>> This is because I felt that if check_refname_format() fails then its a
>>> fatal error while in other cases, it is not as fatal.
>>
>> The name of the command is "check-term-format" and that is precisely
>> its purpose so, from the perspective of the caller, *all* problems
>> with the term are fatal. It's black-and-white, there is no grey:
>> either a term is acceptable, or it isn't; that's all the caller wants
>> to know. Consequently, all problems detected by this function should
>> be reported the same way (preferably via 'return error()').
>
> Sure. I will use 'return error()'. Any particular reason why this
> instead of die() ?This function's specific purpose is *check* some input for validity and let the caller know the result of that check. The most natural way to do so is by returning a success/failure value to the caller.
die() is for signalling an exceptional condition which should abort the program. There is nothing exceptional about a "false" result from check-term-format, thus die() not an appropriate way to pass the result to the caller.