Re: [PATCH v2] send-pack: avoid redundant "pack-objects died with strange error"

From: Johannes Sixt <j6t@kdbg.org>
Date: Oct 16, 2010, 09:25 UTC
Message-ID: <201010161125.33372.j6t@kdbg.org>
In-Reply-To: <20101016060423.GA26538@burratino>

On Samstag, 16. Oktober 2010, Jonathan Nieder wrote:

Show 7 quoted lines

> Saying "pack-objects died with strange error" after "pack-objects
> died of signal 13" seems kind of redundant.  The latter was
> introduced when send-pack switched to the run-command API, which
> reports abnormal exits on behalf of the caller.
>
> Normal exits with nonzero status are not reported by run-command,
> though.

The rationale for this is the assumption that before a program or script exits with non-zero status, it will have reported an error.

Show 5 quoted lines

> Or in the "controlled exit" case:
>
> 	[...]
> 	error: pack-objects died with status 128
> 	error: failed to push some refs to 'ssh://example.com/foo/bar'

I sense that this quote is not complete. Did you strip some error message by writing just "[...]"?

Show 7 quoted lines

> -	if (finish_command(&po))
> -		return error("pack-objects died with strange error");
> -	return 0;
> +	status = finish_command(&po);
> +	if (status > 0)
> +		return error("pack-objects died with status %d", status);
> +	return status;

Ideally, this should really just be

	if (finish_command(&po))
		return -1;

-- Hannes