debuginfod-client.c: Fix download size not correctly fallbacks to x-debuginfod-size header

Message ID 20230329045119.763709-1-lilydjwg@gmail.com
State Superseded
Headers
Series debuginfod-client.c: Fix download size not correctly fallbacks to x-debuginfod-size header |

Commit Message

lilydjwg--- via Elfutils-devel March 29, 2023, 4:51 a.m. UTC
  From: lilydjwg <lilydjwg@gmail.com>

Currently when the debuginfod server doesn't set Content-Length but set
x-debuginfod-size header, it's ignored and the progressfn doesn't know
the total size.

This happens for me with Arch Linux's debuginfod server and gdb.

The reason is that when Content-Length is unavailable, cl is set to -1
by curl but dl_size remains as 0, but later dl_size == -1 is checked.

Signed-off-by: lilydjwg <lilydjwg@gmail.com>
---
 ChangeLog                      | 5 +++++
 debuginfod/debuginfod-client.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)
  

Comments

Frank Ch. Eigler March 29, 2023, 12:28 p.m. UTC | #1
Hi -

> [...]
> The reason is that when Content-Length is unavailable, cl is set to -1
> by curl

Is that behaviour from new versions of curl?

>  but dl_size remains as 0, but later dl_size == -1 is checked.

Or perhaps dl_size needs to be initialized to -1 ("unknown") vs 0
("known to be zero"), and that value mapped to 0 only in the
*c->progressfn() call down below.


- FChE
  
lilydjwg March 29, 2023, 2:57 p.m. UTC | #2
On Wed, Mar 29, 2023 at 08:28:35AM -0400, Frank Ch. Eigler wrote:
> Hi -
> 
> > [...]
> > The reason is that when Content-Length is unavailable, cl is set to -1
> > by curl
> 
> Is that behaviour from new versions of curl?

Yes. curl 8.0.1 to be exact.

> >  but dl_size remains as 0, but later dl_size == -1 is checked.
> 
> Or perhaps dl_size needs to be initialized to -1 ("unknown") vs 0
> ("known to be zero"), and that value mapped to 0 only in the
> *c->progressfn() call down below.

Yes, that can be done. Just more places to be changed.

However, I find a bigger issue just now: the CURLINFO_SIZE_DOWNLOAD_T
counts differently than x-debuginfod-size. I'm sending a v2 patch now.
  
lilydjwg March 29, 2023, 3:05 p.m. UTC | #3
On Wed, Mar 29, 2023 at 10:57:47PM +0800, lilydjwg wrote:
> On Wed, Mar 29, 2023 at 08:28:35AM -0400, Frank Ch. Eigler wrote:
> > Hi -
> > 
> > > [...]
> > > The reason is that when Content-Length is unavailable, cl is set to -1
> > > by curl
> > 
> > Is that behaviour from new versions of curl?
> 
> Yes. curl 8.0.1 to be exact.

To be clear, I mean that I'm having the issue when using curl 8.0.1, not
the value is set to -1. The latter has been like that even in the
"#else" branch which is for older curl.
  

Patch

diff --git a/ChangeLog b/ChangeLog
index 10c23002..05697a02 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@ 
+2023-03-29  lilydjwg  <lilydjwg@gmail.com>
+
+	* debuginfod/debuginfod-client.c: Fix download size not correctly
+	fallbacks to x-debuginfod-size header
+
 2023-03-03  Mark Wielaard  <mark@klomp.org>
 
 	* NEWS: Add ELFCOMPRESS_ZSTD support for libelf and elfcompress.
diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index b33408eb..e494adec 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -1479,7 +1479,7 @@  debuginfod_query_server (debuginfod_client *c,
           curl_res = curl_easy_getinfo(target_handle,
                                        CURLINFO_CONTENT_LENGTH_DOWNLOAD_T,
                                        &cl);
-          if (curl_res == CURLE_OK && cl >= 0)
+          if (curl_res == CURLE_OK)
             dl_size = (cl > LONG_MAX ? LONG_MAX : (long)cl);
 #else
           double cl;