Re: [PATCH] gdb: print size of downloaded debuginfod binary

Tom de Vries
2020-12-12T16:43:53Z

[-- Attachment #1: Type: text/plain, Size: 1607 bytes --]

On 12/11/20 8:23 PM, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
>
> Tom> + gdb::optional<ui_out::progress_meter> meter;
>
> Seems like a bit of a shame to have an optional here, but then
> essentially require optional-style behavior in the CLI ui-out as well
> (in deferring output until the first update).
>

Well, the natural place to declare "ui_out::progress_meter meter" given
its lifetime is in debuginfod_source_query and debuginfod_debuginfo_query.

But the only constructor has the name argument with no means of changing
it later, and the name (containing the size) is only known lateron, when
progressfn is called.

So the optional is used as a means to have lifetime as we want it, while
postponing construction until we known all the constructor arguments.

> Maybe it could be done in progress_meter itself. Not sure.
>
> Tom> - /* Print this message only once. */
> Tom> - data->has_printed = true;
> Tom> - printf_filtered ("Downloading %s %ps...\n",
> Tom> - data->desc,
> Tom> - styled_string (file_name_style.style (), data->fname));
> Tom> + float size_in_mb = 1.0f * total / (1024 * 1024);
> Tom> + std::stringstream message;
> Tom> + message.precision (2);
> Tom> + message << "Downloading " << size_in_mb << " MB " << data->desc
> Tom> + << " " << data->fname;
>
> Seems like this could use string_printf.

Done.

> I suspect there's some way to preserve the styling but offhand I don't
> recall what it is. Maybe you have to use string_file to accomplish that.
>

Done.

Thanks,
- Tom

[-- Attachment #2: 0011-gdb-Print-progress-for-debuginfod.patch --]
[-- Type: text/x-patch, Size: 2602 bytes --]

[gdb] Print progress for debuginfod

Prints progress like:

Downloading 4.89 MB separate debug info for /usr/lib64/libgcrypt.so.20.
Downloading 1.10 MB separate debug info for /usr/lib64/liblzma.so.5.
Downloading 1.31 MB separate debug info for /usr/lib64/liblz4.so.1.
Downloading 0.96 MB separate debug info for /usr/lib64/libsmime3.so.
[### ]

Tested on x86_64-linux.

ChangeLog:

2020-12-10 Martin Liska <mliska@suse.cz>
Tom de Vries <tdevries@suse.de>

* gdb/debuginfod-support.c (struct user_data): Remove has_printed
field. Add meter field.
(progressfn): Print progress using meter.

---
gdb/debuginfod-support.c | 27 +++++++++++++++++++--------
1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c
index e21b2f40ca..e9b2dcd5be 100644
--- a/gdb/debuginfod-support.c
+++ b/gdb/debuginfod-support.c
@@ -21,6 +21,7 @@
#include "cli/cli-style.h"
#include "gdbsupport/scoped_fd.h"
#include "debuginfod-support.h"
+#include "gdbsupport/gdb_optional.h"

#ifndef HAVE_LIBDEBUGINFOD
scoped_fd
@@ -46,12 +47,12 @@ debuginfod_debuginfo_query (const unsigned char *build_id,
struct user_data
{
user_data (const char *desc, const char *fname)
- : desc (desc), fname (fname), has_printed (false)
+ : desc (desc), fname (fname)
{ }

const char * const desc;
const char * const fname;
- bool has_printed;
+ gdb::optional<ui_out::progress_meter> meter;
};

/* Deleter for a debuginfod_client. */
@@ -80,15 +81,25 @@ progressfn (debuginfod_client *c, long cur, long total)
return 1;
}

- if (!data->has_printed && total != 0)
+ if (total == 0)
+ return 0;
+
+ if (!data->meter.has_value ())
{
- /* Print this message only once. */
- data->has_printed = true;
- printf_filtered ("Downloading %s %ps...\n",
- data->desc,
- styled_string (file_name_style.style (), data->fname));
+ float size_in_mb = 1.0f * total / (1024 * 1024);
+ string_file styled_filename (current_uiout->can_emit_style_escape ());
+ fprintf_styled (&styled_filename,
+ file_name_style.style (),
+ "%s",
+ data->fname);
+ std::string message
+ = string_printf ("Downloading %.2f MB %s %s", size_in_mb, data->desc,
+ styled_filename.c_str());
+ data->meter.emplace (current_uiout, message, 1);
}

+ current_uiout->progress ((double)cur / (double)total);
+
return 0;
}