From: Eric Sunshine Date: Wed, 04 May 2016 17:49:39 GMT Subject: Re: [PATCH 2/2] bisect: rewrite `check_term_format` shell function in C Message-ID: In-Reply-To: On Wed, May 4, 2016 at 7:58 AM, Pranit Bauva wrote: > On Wed, May 4, 2016 at 1:58 PM, Eric Sunshine wrote: >> On Wed, May 4, 2016 at 3:36 AM, Pranit Bauva wrote: >>> On Wed, May 4, 2016 at 12:22 PM, Eric Sunshine wrote: >>>> On Wed, May 4, 2016 at 1:07 AM, Pranit Bauva 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.