Re: [PATCH v2] config: Use parseopt.

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 15, 2009, 19:36 UTC
Message-ID: <7v1vtzlcfi.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <alpine.DEB.1.00.0902142328530.10279@pacific.mpi-cbg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 9 quoted lines

> On Sat, 14 Feb 2009, Felipe Contreras wrote:
>> On Sat, Feb 14, 2009 at 9:59 PM, Johannes Schindelin
>> <Johannes.Schindelin@gmx.de> wrote:
>> > Or maybe even better: set a variable "ret" and at the end of 
>> > cmd_config(), "return !!ret;"?
>> 
>> Huh? So git commands don't return negative error values?
>
> AFAICT an exit status is supposed to be between 0 and 127.

Are you two talking about the return value from cmd_config()?

git.c::run_builtin() already knows what to do with status codes from the builtins to protect you from (rare) shells that do not cope with a negative return that come from the common pattern of doing:

	return error("it is wrong in this way")

So "negative" is not really a problem. See 2488df8 (builtin run_command: do not exit with -1., 2007-11-13).

But if the old code was doing:

	ret = git_config_set_multivar(...);
        if (ret)
		return ret;

and if you are changing it to:

	ret = git_config_set_multivar(...);
        if (ret)
		return !!ret;

you are changing an externally observable behaviour.