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 7D20F385354D for ; Tue, 6 Sep 2022 15:49:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7D20F385354D 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 0E615302AB2C; Tue, 6 Sep 2022 17:49:28 +0200 (CEST) Received: by tarox.wildebeest.org (Postfix, from userid 1000) id C686C4016579; Tue, 6 Sep 2022 17:49:27 +0200 (CEST) Message-ID: Subject: Re: [patch git] PR28284 - debuginfod x-debuginfod* header processing From: Mark Wielaard To: "Frank Ch. Eigler" , elfutils-devel@sourceware.org Date: Tue, 06 Sep 2022 17:49:27 +0200 In-Reply-To: <20220903001304.GA20286@redhat.com> References: <20220903001304.GA20286@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=-5.3 required=5.0 tests=BAYES_00,JMQ_SPF_NEUTRAL,KAM_DMARC_STATUS,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE 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 Frank, On Fri, 2022-09-02 at 20:13 -0400, Frank Ch. Eigler via Elfutils-devel wrote: > I had a bit of time to tweak Noah Sanci's PR28284 work, and I believe > it addresses Mark's last set of comments (from a while ago). This > follow-up patch corrects things like case sensitivity, spacing, \r\n > processing, and tweaks documentation. I hadn't thought about the \r\n at the end of the HTTP headers. Thanks. I assume \r\n at the end of HTTP headers required, since you are expecting in the code now, or could it also be \n on its own? > The gist of it is to add a new client api function > debuginfod_get_headers(), documented to return x-debuginfod* headers > from current or previous fetch requests. This looks good, but I think c->winning_headers needs to be freed/cleared at the start of debuginfod_query_server. Otherwise if you reuse the debuginfod_client and you hit the cache, the user gets the headers from the last use of debuginfod_client that did fetch something from a server. Which imho is confusing (the headers won't match the cached result returned). > debuginfod-find prints those > in -v verbose mode, and debuginfod relays them in federation. This is the only thing I am not 100% happy about. It means you can only see the headers using debuginfod-find but no longer with any other client when DEBUGINFOD_VERBOSE is set. Is this really what we want? > This stuff is an enabler for rgoldber's subsequent > signature-passing/checking code, to which I plan to turn my attention > next. >=20 > Please see users/fche/try-pr28284d for this draft of the code. I'd > like to keep it as two separate commits to preserve Noah's id in the > git history, even though that makes it a bit harder to give a final > review. Thanks. I like this version except for those two nitpicks above. What do you think? Cheers, Mark