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: AS3215 2.6.0.0/16 X-Spam-Status: No, score=-0.5 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20]) by dcvr.yhbt.net (Postfix) with ESMTP id 426281F4D7 for ; Thu, 16 Jun 2022 04:23:15 +0000 (UTC) Authentication-Results: dcvr.yhbt.net; dkim=pass (1024-bit key; unprotected) header.d=pobox.com header.i=@pobox.com header.b="QeqegSBg"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238847AbiFPEXK (ORCPT ); Thu, 16 Jun 2022 00:23:10 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35496 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229485AbiFPEXI (ORCPT ); Thu, 16 Jun 2022 00:23:08 -0400 Received: from pb-smtp2.pobox.com (pb-smtp2.pobox.com [64.147.108.71]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 022182C11C for ; Wed, 15 Jun 2022 21:23:07 -0700 (PDT) Received: from pb-smtp2.pobox.com (unknown [127.0.0.1]) by pb-smtp2.pobox.com (Postfix) with ESMTP id 34ADC124220; Thu, 16 Jun 2022 00:23:06 -0400 (EDT) (envelope-from junio@pobox.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=pobox.com; h=from:to:cc :subject:references:date:in-reply-to:message-id:mime-version :content-type; s=sasl; bh=Ig+74Sgdh4ckhgrbaQBkRnJPLGSsXl5JxaQ3gK qAxDk=; b=QeqegSBgh2GJapMPAGbJBgQG/L5Yry687h1dtod+rShWVhT/NLksOU fRa5AtrhXDFmDqfORfKvY4LZYvnLkMxB1HYS2mwqL8mXPE78SR2Xs1fLu9tNQng9 U4JPIMA/R9xbl+21LfrrGbMP8TpCtHka3cr0eddNxptWIH+F/Qqjs= Received: from pb-smtp2.nyi.icgroup.com (unknown [127.0.0.1]) by pb-smtp2.pobox.com (Postfix) with ESMTP id 2BF1812421E; Thu, 16 Jun 2022 00:23:06 -0400 (EDT) (envelope-from junio@pobox.com) Received: from pobox.com (unknown [34.82.80.254]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by pb-smtp2.pobox.com (Postfix) with ESMTPSA id 8958812421B; Thu, 16 Jun 2022 00:23:05 -0400 (EDT) (envelope-from junio@pobox.com) From: Junio C Hamano To: "Johannes Schindelin via GitGitGadget" Cc: git@vger.kernel.org, Johannes Schindelin , Glen Choo Subject: Re: [PATCH 03/11] submodule--helper: avoid memory leak in `update_submodule()` References: <877a45867ae368bf9e053caedcb6cf421e02344d.1655336146.git.gitgitgadget@gmail.com> Date: Wed, 15 Jun 2022 21:23:04 -0700 In-Reply-To: <877a45867ae368bf9e053caedcb6cf421e02344d.1655336146.git.gitgitgadget@gmail.com> (Johannes Schindelin via GitGitGadget's message of "Wed, 15 Jun 2022 23:35:37 +0000") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Pobox-Relay-ID: 04C180CA-ED2C-11EC-B7E4-CB998F0A682E-77302942!pb-smtp2.pobox.com Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org "Johannes Schindelin via GitGitGadget" writes: > From: Johannes Schindelin > > Reported by Coverity. > > Signed-off-by: Johannes Schindelin > --- > builtin/submodule--helper.c | 2 ++ > 1 file changed, 2 insertions(+) > diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c > index 5c77dfcffee..d7b8004b933 100644 > --- a/builtin/submodule--helper.c > +++ b/builtin/submodule--helper.c > @@ -2512,6 +2512,8 @@ static int update_submodule(struct update_data *update_data) > > next.recursive_prefix = get_submodule_displaypath(prefixed_path, > update_data->prefix); > + free(prefixed_path); > + This function has two very similar code block that computes prefixed_path depending on the same condition, and one frees the variable correctly while the other one (i.e. this one) forgets to do so, which is irritating to see. Perhaps the whole "we have update_data structure, in which recursive_prefix, sm_path and prefix members in it; please set the displaypath member based on these values" should become a helper function, e.g. static const char *displaypath_from_update_data(struct update_data *u) { char *pp, *ret; if (u->recursive_prefix) pp = xstrfmt("%s%s", u->recursive_prefix, u->sm_path); else pp = xstrdup(u->sm_path); ret = get_submodule_displaypath(pp, u->prefix); free(pp); return ret; } to avoid duplicated computation. But the whole thing may become moot, as there seems to be a move to get rid of submodule--helper.c altogether? I'll refrain from touching this patch and instead redirect it to Glen; perhaps removal of submodule--helper.c involves moving the code here to another file or something, in which case it is far easier if I outsource that to somebody who is actually working on the file ;-) Thanks. > next.prefix = NULL; > oidcpy(&next.oid, null_oid()); > oidcpy(&next.suboid, null_oid());