From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gnu.wildebeest.org (gnu.wildebeest.org [45.83.234.184]) by sourceware.org (Postfix) with ESMTPS id 4F63A38582A2 for ; Mon, 7 Nov 2022 14:19:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4F63A38582A2 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=klomp.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=klomp.org Received: from tarox.wildebeest.org (83-87-18-245.cable.dynamic.v4.ziggo.nl [83.87.18.245]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by gnu.wildebeest.org (Postfix) with ESMTPSA id 22EE73045FB4; Mon, 7 Nov 2022 15:19:42 +0100 (CET) Received: by tarox.wildebeest.org (Postfix, from userid 1000) id 194C4413CBBB; Mon, 7 Nov 2022 15:19:41 +0100 (CET) Message-ID: Subject: Re: [COMMITTED] debuginfod_find_section: Always update rc with most recent error code From: Mark Wielaard To: Aaron Merey , elfutils-devel@sourceware.org Date: Mon, 07 Nov 2022 15:19:41 +0100 In-Reply-To: <20221104214018.718373-1-amerey@redhat.com> References: <20221104214018.718373-1-amerey@redhat.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Mailer: Evolution 3.28.5 (3.28.5-10.el7) Mime-Version: 1.0 X-Spam-Status: No, score=-3039.1 required=5.0 tests=BAYES_00,GIT_PATCH_0,JMQ_SPF_NEUTRAL,KAM_DMARC_STATUS,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi Aaron, On Fri, 2022-11-04 at 17:40 -0400, Aaron Merey via Elfutils-devel wrote: > debuginfod_find_section may attempt to download both the debuginfo > and executable matching the given build-id. If neither of these > files can be found, update rc to ensure that we always return an > accurate error code in this case. Nicely spotted, otherwise we might have returned -EEXIST. > diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod- > client.c > index f48e32cc..99da05ef 100644 > --- a/debuginfod/debuginfod-client.c > +++ b/debuginfod/debuginfod-client.c > @@ -1944,7 +1944,8 @@ debuginfod_find_section (debuginfod_client *client, > =20 > if (rc =3D=3D -EEXIST) > { > - /* The section should be found in the executable. */ > + /* Either the debuginfo couldn't be found or the section should > + be in the executable. */ > fd =3D debuginfod_find_executable (client, build_id, > build_id_len, &tmp_path); > if (fd > 0) I know this is in existing code, so this might have missed in a previous review. But shouldn't this be fd >=3D 0 ? That is what is checked in the rest of the code. Except for the debuginfod_find_section function which uses fd >0 twice. It is unlikely, but I think fd can be zero if it (stdin) was closed by the program for some reason. Then I think zero can be reused as new file descriptor? Cheers, Mark