From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS31976 209.132.180.0/23 X-Spam-Status: No, score=-4.0 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI,RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by dcvr.yhbt.net (Postfix) with ESMTP id D5B551F466 for ; Fri, 31 Jan 2020 18:31:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726336AbgAaSbt (ORCPT ); Fri, 31 Jan 2020 13:31:49 -0500 Received: from pb-smtp1.pobox.com ([64.147.108.70]:61500 "EHLO pb-smtp1.pobox.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725978AbgAaSbs (ORCPT ); Fri, 31 Jan 2020 13:31:48 -0500 Received: from pb-smtp1.pobox.com (unknown [127.0.0.1]) by pb-smtp1.pobox.com (Postfix) with ESMTP id 8461D3B6DA; Fri, 31 Jan 2020 13:31:43 -0500 (EST) (envelope-from junio@pobox.com) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=pobox.com; h=from:to:cc :subject:references:date:in-reply-to:message-id:mime-version :content-type; s=sasl; bh=4VTo/DDUi1DpgtADhpgFVilA3Zc=; b=D3DJWD 5Fx7QfoOGBUEnQMMxBTWDScHDGA+FGsGqxffpOSme0Ncte4y8eY5qQG/cnh50PxF pED1L34+LlecibfZqFFRzu4NyGiPWkArF7uaxCSpLOzyzSi7F62Rk0VGPDff4yrE 0vYE9JPaqTrDXoI0SzgmRSkFUFkzlaGXBaXxo= DomainKey-Signature: a=rsa-sha1; c=nofws; d=pobox.com; h=from:to:cc :subject:references:date:in-reply-to:message-id:mime-version :content-type; q=dns; s=sasl; b=ALSPKDTfhOYmDnUfw4gz7dxfVGZO11A8 PATsl7jlYz8Vlf0QVa0e+wkXDoaVBktY5TFC7gKpV/tVdBD6RmfbopZeE11ub0Fk aFwl194YZ3+5Jy4AB+2nVQpQDoUN9od3S0+A4oFMVVrAZr0jTUl5xfSvc00K2etl mXz4DAZwv0Y= Received: from pb-smtp1.nyi.icgroup.com (unknown [127.0.0.1]) by pb-smtp1.pobox.com (Postfix) with ESMTP id 7B8E83B6D9; Fri, 31 Jan 2020 13:31:43 -0500 (EST) (envelope-from junio@pobox.com) Received: from pobox.com (unknown [34.76.80.147]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by pb-smtp1.pobox.com (Postfix) with ESMTPSA id D0D243B6D8; Fri, 31 Jan 2020 13:31:42 -0500 (EST) (envelope-from junio@pobox.com) From: Junio C Hamano To: Miriam Rubio Cc: git@vger.kernel.org, Pranit Bauva , Christian Couder , Johannes Schindelin , Tanushree Tumane Subject: Re: [PATCH v2 07/11] bisect: libify `bisect_checkout` References: <20200128144026.53128-1-mirucam@gmail.com> <20200128144026.53128-8-mirucam@gmail.com> Date: Fri, 31 Jan 2020 10:31:41 -0800 In-Reply-To: <20200128144026.53128-8-mirucam@gmail.com> (Miriam Rubio's message of "Tue, 28 Jan 2020 15:40:22 +0100") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Pobox-Relay-ID: EDAF072E-4457-11EA-8F0C-C28CBED8090B-77302942!pb-smtp1.pobox.com Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Miriam Rubio writes: > From: Pranit Bauva > > Since we want to get rid of git-bisect.sh it would be necessary to > convert those exit() calls to return statements so that errors can be > reported. > > Emulate try catch in C by converting `exit()` to > `return `. Follow POSIX conventions to return > to indicate error. > > Turn `exit()` to `return` calls in `bisect_checkout()`. > Changes related to return values have no bad side effects on the > code that calls `bisect_checkout()`. > > Mentored-by: Christian Couder > Mentored-by: Johannes Schindelin > Signed-off-by: Pranit Bauva > Signed-off-by: Tanushree Tumane > Signed-off-by: Miriam Rubio > --- > bisect.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/bisect.c b/bisect.c > index a7a5d158e6..dee8318d9b 100644 > --- a/bisect.c > +++ b/bisect.c > @@ -713,6 +713,7 @@ static int bisect_checkout(const struct object_id *bisect_rev, int no_checkout) > { > char bisect_rev_hex[GIT_MAX_HEXSZ + 1]; > > + int res = 0; > memcpy(bisect_rev_hex, oid_to_hex(bisect_rev), the_hash_algo->hexsz + 1); Wrong placement of a new decl. Have a block of decls and then have a blank line before the first statement, i.e. char bisect_rev_hex[...]; + int res = 0; memcpy(...); This comment probably applies to other hunks in the entire series. > @@ -721,14 +722,14 @@ static int bisect_checkout(const struct object_id *bisect_rev, int no_checkout) > update_ref(NULL, "BISECT_HEAD", bisect_rev, NULL, 0, > UPDATE_REFS_DIE_ON_ERR); > } else { > - int res; > res = run_command_v_opt(argv_checkout, RUN_GIT_CMD); > if (res) > - exit(res); > + return res > 0 ? -res : res; Hmph. This means that res == -1 and res == 1 from run_command_v_opt() cannot be distinguished by our callers. Is that what we want here? If that is really what we want, it probably is easier to read if this were written like so: return -abs(res); > argv_show_branch[1] = bisect_rev_hex; > - return run_command_v_opt(argv_show_branch, RUN_GIT_CMD); > + res = run_command_v_opt(argv_show_branch, RUN_GIT_CMD); > + return res > 0 ? -res : res; Likewise.