From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gnu.wildebeest.org (wildebeest.demon.nl [212.238.236.112]) by sourceware.org (Postfix) with ESMTPS id 1E25D3857000 for ; Sun, 6 Dec 2020 13:20:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 1E25D3857000 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=klomp.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mark@klomp.org Received: from tarox.wildebeest.org (tarox.wildebeest.org [172.31.17.39]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by gnu.wildebeest.org (Postfix) with ESMTPSA id 05EE837251CA; Sun, 6 Dec 2020 14:20:11 +0100 (CET) Received: by tarox.wildebeest.org (Postfix, from userid 1000) id 77D18413CB2B; Sun, 6 Dec 2020 14:20:11 +0100 (CET) Message-ID: <633e97a4293852732af8a9edba9b383a5f9cfeb1.camel@klomp.org> Subject: Re: [PATCH 1/3] link_map: Pull release_buffer() into file scope From: Mark Wielaard To: tbaeder@redhat.com, elfutils-devel@sourceware.org Date: Sun, 06 Dec 2020 14:20:11 +0100 In-Reply-To: <20201201083854.1870943-2-tbaeder@redhat.com> References: <20201201083854.1870943-1-tbaeder@redhat.com> <20201201083854.1870943-2-tbaeder@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=-11.5 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.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: elfutils-devel@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Elfutils-devel mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 06 Dec 2020 13:20:21 -0000 Hi Timm, On Tue, 2020-12-01 at 09:38 +0100, Timm B=C3=A4der via Elfutils-devel wrote= : > From: Timm B=C3=A4der >=20 > Get rid of another nested function It is missing a ChangeLog entry. > diff --git a/libdwfl/link_map.c b/libdwfl/link_map.c > index 29307c74..5c39c631 100644 > --- a/libdwfl/link_map.c > +++ b/libdwfl/link_map.c > @@ -225,6 +225,18 @@ addrsize (uint_fast8_t elfclass) > return elfclass * 4; > } > =20 > +static inline int > +release_buffer (Dwfl *dwfl, > + Dwfl_Memory_Callback *memory_callback, void *memory_call= back_arg, > + void **buffer, size_t *buffer_available, > + int result) > +{ > + if (buffer !=3D NULL) > + (void) (*memory_callback) (dwfl, -1, buffer, buffer_available, 0, 0, > + memory_callback_arg); > + return result; > +} Note that this changes the semantics slightly. Because you now take the address of the buffer variable before checking it is NULL (it now never is). Also note that the result is not always used, so it might be cleaner to not pass it around. It is IMHO better to fully inline it. > @@ -249,13 +261,6 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t = elfdata, > =20 > void *buffer =3D NULL; > size_t buffer_available =3D 0; > - inline int release_buffer (int result) > - { > - if (buffer !=3D NULL) > - (void) (*memory_callback) (dwfl, -1, &buffer, &buffer_available, 0= , 0, > - memory_callback_arg); > - return result; > - } > =20 > GElf_Addr addrs[4]; > inline bool read_addrs (GElf_Addr vaddr, size_t n) > [...] > @@ -304,7 +310,9 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t e= lfdata, > } > =20 > if (unlikely (read_addrs (read_vaddr, 1))) > - return release_buffer (-1); > + release_buffer (dwfl, memory_callback, memory_callback_arg, > + &buffer, &buffer_available, -1); > + Note that here the result is used, but the change doesn't use it anymore and so doesn't return early but just tries to carry on after an error occurred. Cheers, Mark