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 482713857404 for ; Thu, 31 Mar 2022 17:38:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 482713857404 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 13A9A304319C; Thu, 31 Mar 2022 19:38:02 +0200 (CEST) Received: by tarox.wildebeest.org (Postfix, from userid 1000) id 1949D413CD0E; Thu, 31 Mar 2022 19:38:00 +0200 (CEST) Message-ID: <9e72049a6e8b9a964d68cfc70b98aea682911b13.camel@klomp.org> Subject: Re: [PATCH] [PATCH] debuginfod: Use the debuginfod-size response header From: Mark Wielaard To: Aaron Merey , elfutils-devel@sourceware.org Date: Thu, 31 Mar 2022 19:37:59 +0200 In-Reply-To: <20220112030755.434016-1-amerey@redhat.com> References: <20220112030755.434016-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=-3.8 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.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Thu, 31 Mar 2022 17:38:07 -0000 Hi Aaron, On Tue, 2022-01-11 at 22:07 -0500, Aaron Merey via Elfutils-devel wrote: > In some cases the content-length header may not be available in order > to pass to a progressfn. If content-length isn't available then attempt > to get the size of the download from the debuginfod-size header instead. >=20 > It should be mentioned that if a compressed file (ex. gzip) is being > transferred, the actual transfer length will be less than debuginfod-size= . > In this case debuginfod-size is a best-guess upper bound on the size of > the transfer. Sorry this patch wasn't reviewed for such a long time. It looks correct and is also a nice cleanup. Consolidating getting the download size in one place instead of two (for the progress function and the maxsize check). Just a question about this part: > + /* If Content-Length is -1, try to get the size from > + X-Debuginfod-Size */ > + if (dl_size =3D=3D -1 && c->winning_headers !=3D NULL) > + { > + double xdl; > + char *hdr =3D strstr(c->winning_headers, "x-debuginfod-siz= e"); > + > + if (hdr !=3D NULL > + && sscanf(hdr, "x-debuginfod-size: %lf", &xdl) =3D=3D = 1) > + dl_size =3D (xdl >=3D (double)(LONG_MAX+1UL) ? LONG_MAX = : (long)xdl); > + } > + } In debuginfod.cxx the header is spelled all uppercase as "X-DEBUGINFOD- SIZE" which is also what is checked for in the run-debuginfod-response- headers.sh test. So shouldn't the above also be all uppercase or should you use strcasestr? When using sscanf why are you using a double and %lf? Isn't it simpler to use a long and %ld? Is there a way to test this easily? When would Content-Length not be available? Cheers, Mark