Re: [PATCH] debuginfod-support.c: Replace globals with user_data

[Simon Marchi <simark@simark.ca>]

> From 1913317cb95d02c3244e5d91b7b59e33cfe991c5 Mon Sep 17 00:00:00 2001
> From: Aaron Merey <amerey@redhat.com>
> Date: Fri, 31 Jul 2020 18:25:48 -0400
> Subject: [PATCH] debuginfod-support.c: Replace globals with user_data
>
> Store query information in user_data struct instead of global
> variables. Also include DEBUGINFOD_CFLAGS in INTERNAL_CFLAG_BASE.

The DEBUGINFOD_CFLAGS change looks good, but it's unrelated to the topic of this patch,
please make it its own patch, with its own justification.

> diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c
> index f4a227b040..9325bf8651 100644
> --- a/gdb/debuginfod-support.c
> +++ b/gdb/debuginfod-support.c
> @@ -43,29 +43,39 @@ debuginfod_debuginfo_query (const unsigned char *build_id,
>  #else
>  #include <elfutils/debuginfod.h>
>
> -/* TODO: Use debuginfod API extensions instead of these globals.  */
> -static std::string desc;
> -static std::string fname;
> -static bool has_printed;
> +struct user_data
> +{
> +  std::string desc;
> +  std::string fname;
> +  bool has_printed;
> +
> +  user_data (std::string desc, std::string fname, bool has_printed)
> +    : desc (desc), fname (fname), has_printed (has_printed)
> +  { }

Put the constructor before the fields.  Make the fields const if they are not intended
to be changed once the object has been created.

Using `std::string` causes unnecessary copying to happen.  Just use `const char *`
(`const char * const` for the fields) or gdb::string_view, if you want to be more
generic (but all callers are `const char *`, so it's not really necessary).  That's
assuming that the lifetime of those strings will always be longer than the lifetime
of the debuginfo client.  I think that's the case, since the debug info client is
always created and destroyed in the same function.

> +};
>
>  static int
>  progressfn (debuginfod_client *c, long cur, long total)
>  {
> +  struct user_data *data = static_cast<user_data*> (debuginfod_get_user_data (c));

This line is too long, just skip the `struct` keyword and you'll be fine.  Also, space
before `*`.

> +
>    if (check_quit_flag ())
>      {
>        printf_filtered ("Cancelling download of %s %ps...\n",
> -		       desc.c_str (),
> -		       styled_string (file_name_style.style (), fname.c_str ()));
> +		       data->desc.c_str (),
> +		       styled_string (file_name_style.style (),
> +				      data->fname.c_str ()));
>        return 1;
>      }
>
> -  if (!has_printed && total != 0)
> +  if (! data->has_printed && total != 0)

Remove space after `!`.

Simon