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 B2D2938582AF for ; Tue, 1 Nov 2022 20:29:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B2D2938582AF 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 reform (deer0x15.wildebeest.org [172.31.17.151]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by gnu.wildebeest.org (Postfix) with ESMTPSA id 7B6323061FA5; Tue, 1 Nov 2022 21:29:14 +0100 (CET) Received: by reform (Postfix, from userid 1000) id 016172E81273; Tue, 1 Nov 2022 21:29:13 +0100 (CET) Date: Tue, 1 Nov 2022 21:29:13 +0100 From: Mark Wielaard To: Aaron Merey Cc: elfutils-devel@sourceware.org Subject: Re: [PATCH v3] debuginfod: Support queries for ELF/DWARF sections Message-ID: References: <20221101045341.88982-1-amerey@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20221101045341.88982-1-amerey@redhat.com> X-Spam-Status: No, score=-3033.0 required=5.0 tests=BAYES_00,JMQ_SPF_NEUTRAL,KAM_DMARC_STATUS,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=no 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 Tue, Nov 01, 2022 at 12:53:41AM -0400, Aaron Merey wrote: > Thanks again for the detailed review. I fixed the issues you pointed out. This version looks really good. Please push, so it is included in the release tomorrow. > On Sat, Oct 29, 2022 at 8:29 PM Mark Wielaard wrote: > > > @@ -1008,6 +1206,9 @@ debuginfod_query_server (debuginfod_client *c, > > >            snprintf(data[i].url, PATH_MAX, "%s/%s/%s/%s", server_url, > > >                     build_id_bytes, type, escaped_string); > > >          } > > > +      else if (section) > > > +     snprintf(data[i].url, PATH_MAX, "%s/%s/%s/%s", server_url, > > > +              build_id_bytes, type, section); > > > > Does the section part need to be path escaped? Like the filename is > > with curl_easy_escape? And if it is how does that interact with the > > cache file name? > > I assumed that section names would not contain '/'. However I noticed > that if we call debuginfod_find_section with a section name containing > '/', it will return -EINVAL. This triggers the downloading of debuginfo > in order to attempt client-side extraction. This is a waste so I changed > the client to path-escape the section name for caching purposes just like > the source query filename. It is highly unlikely a '/' is in the section name, but good to have this covered anyway. Nicely extracted that escape function. > > > +  /* The servers may have lacked support for section queries.  Attempt to > > > +     download the debuginfo or executable containing the section in order > > > +     to extract it.  */ > > > > This does somewhat defeat the purpose of just getting the section data > > of course. So we could also just return EINVAL to the user here, so > > they have to get the whole debuginfo themselves. But maybe just > > pretending it worked is fine to keep the code path of the user > > simpler? > > This keeps the code path simpler for the user and like Frank mentioned > it enables an intermediate server to extract the section and provide > it to the client. This server will also cache the debuginfo/executable, > possibly speeding up future requests. Ah, yes, the debuginfod server doesn use the client library too. Thanks, Mark