Re: [PATCH v2] Use styled_string when defering warnings when loading separate debug files

[Alexandra Petlanova Hajkova <ahajkova@redhat.com>]

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

Ping

On Mon, Mar 20, 2023 at 3:56 PM Alexandra Hájková via Gdb-patches <
gdb-patches@sourceware.org> wrote:

> This improves commit 6647f05df023b63bbe056e9167e9e234172fa2ca, the
> filenames used in the warnings are styled properly now.
> ---
> This version uses ui_file rather instead of vector of callback functions
> that emit warnings.
> Addresses various cosmetic issues.
>
>  gdb/build-id.c | 16 ++++++++++------
>  gdb/build-id.h |  4 ++--
>  gdb/coffread.c |  4 ++--
>  gdb/elfread.c  |  4 ++--
>  gdb/symfile.c  | 26 ++++++++++++++++----------
>  gdb/symfile.h  |  4 ++--
>  gdb/ui-file.h  |  7 +++++++
>  7 files changed, 41 insertions(+), 24 deletions(-)
>
> diff --git a/gdb/build-id.c b/gdb/build-id.c
> index 00250c20ae9..5d5114e2ace 100644
> --- a/gdb/build-id.c
> +++ b/gdb/build-id.c
> @@ -18,6 +18,7 @@
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.
> */
>
>  #include "defs.h"
> +#include "cli/cli-style.h"
>  #include "bfd.h"
>  #include "gdb_bfd.h"
>  #include "build-id.h"
> @@ -209,7 +210,7 @@ build_id_to_exec_bfd (size_t build_id_len, const
> bfd_byte *build_id)
>
>  std::string
>  find_separate_debug_file_by_buildid (struct objfile *objfile,
> -                                    std::vector<std::string>
> *warnings_vector)
> +                                    std::vector<string_file>
> *warnings_vector)
>  {
>    const struct bfd_build_id *build_id;
>
> @@ -228,12 +229,15 @@ find_separate_debug_file_by_buildid (struct objfile
> *objfile,
>           && filename_cmp (bfd_get_filename (abfd.get ()),
>                            objfile_name (objfile)) == 0)
>         {
> -         std::string msg
> -           = string_printf (_("\"%s\": separate debug info file has no "
> -                              "debug info"), bfd_get_filename (abfd.get
> ()));
> +         string_file warnings (true);
> +         warnings.printf ("\"%ps\":separate debug info file has no debug
> info",
> +                          styled_string (file_name_style.style (),
> +                                         bfd_get_filename (abfd.get ())));
>           if (separate_debug_file_debug)
> -           gdb_printf (gdb_stdlog, "%s", msg.c_str ());
> -         warnings_vector->emplace_back (std::move (msg));
> +           /* Avoid styling for the debug output.  */
> +           gdb_printf (gdb_stdlog, _("%s: separate debug info file has no
> debug info"),
> +                       bfd_get_filename (abfd.get ()));
> +         warnings_vector->emplace_back (std::move (warnings));
>         }
>        else if (abfd != NULL)
>         return std::string (bfd_get_filename (abfd.get ()));
> diff --git a/gdb/build-id.h b/gdb/build-id.h
> index 191720ddf28..0c2c4181b4e 100644
> --- a/gdb/build-id.h
> +++ b/gdb/build-id.h
> @@ -52,13 +52,13 @@ extern gdb_bfd_ref_ptr build_id_to_exec_bfd (size_t
> build_id_len,
>     separate debug file, otherwise, return an empty string.
>
>     Any warnings that are generated by the lookup process should be added
> to
> -   WARNINGS_VECTOR, one std::string per warning.  If some other mechanism
> can
> +   WARNINGS_VECTOR, one string_file per warning.  If some other mechanism
> can
>     be used to lookup the debug information then the warning will not be
> shown,
>     however, if GDB fails to find suitable debug information using any
>     approach, then any warnings will be printed.  */
>
>  extern std::string find_separate_debug_file_by_buildid
> -  (struct objfile *objfile, std::vector<std::string> *warnings_vector);
> +  (struct objfile *objfile, std::vector<string_file> *warnings_vector);
>
>  /* Return an hex-string representation of BUILD_ID.  */
>
> diff --git a/gdb/coffread.c b/gdb/coffread.c
> index e993b17db09..32705c215fd 100644
> --- a/gdb/coffread.c
> +++ b/gdb/coffread.c
> @@ -729,7 +729,7 @@ coff_symfile_read (struct objfile *objfile,
> symfile_add_flags symfile_flags)
>    /* Try to add separate debug file if no symbols table found.   */
>    if (!objfile->has_partial_symbols ())
>      {
> -      std::vector<std::string> warnings_vector;
> +      std::vector<string_file> warnings_vector;
>        std::string debugfile
>         = find_separate_debug_file_by_buildid (objfile, &warnings_vector);
>
> @@ -747,7 +747,7 @@ coff_symfile_read (struct objfile *objfile,
> symfile_add_flags symfile_flags)
>        /* If all the methods to collect the debuginfo failed, print any
>          warnings that were collected.  */
>        if (debugfile.empty () && !warnings_vector.empty ())
> -       for (const std::string &w : warnings_vector)
> +       for (const string_file &w : warnings_vector)
>           warning ("%s", w.c_str ());
>      }
>  }
> diff --git a/gdb/elfread.c b/gdb/elfread.c
> index b414da9ed21..4bc89d09d65 100644
> --- a/gdb/elfread.c
> +++ b/gdb/elfread.c
> @@ -1217,7 +1217,7 @@ elf_symfile_read_dwarf2 (struct objfile *objfile,
>            && objfile->separate_debug_objfile == NULL
>            && objfile->separate_debug_objfile_backlink == NULL)
>      {
> -      std::vector<std::string> warnings_vector;
> +      std::vector<string_file> warnings_vector;
>
>        std::string debugfile
>         = find_separate_debug_file_by_buildid (objfile, &warnings_vector);
> @@ -1269,7 +1269,7 @@ elf_symfile_read_dwarf2 (struct objfile *objfile,
>        /* If all the methods to collect the debuginfo failed, print
>          the warnings, if there're any. */
>        if (debugfile.empty () && !has_dwarf2 && !warnings_vector.empty ())
> -       for (const std::string &w : warnings_vector)
> +       for (const string_file &w : warnings_vector)
>           warning ("%s", w.c_str ());
>      }
>
> diff --git a/gdb/symfile.c b/gdb/symfile.c
> index bb9981a4634..64d29ea71f9 100644
> --- a/gdb/symfile.c
> +++ b/gdb/symfile.c
> @@ -1244,7 +1244,7 @@ bool separate_debug_file_debug = false;
>  static int
>  separate_debug_file_exists (const std::string &name, unsigned long crc,
>                             struct objfile *parent_objfile,
> -                           std::vector<std::string> *warnings_vector)
> +                           std::vector<string_file> *warnings_vector)
>  {
>    unsigned long file_crc;
>    int file_crc_p;
> @@ -1337,13 +1337,19 @@ separate_debug_file_exists (const std::string
> &name, unsigned long crc,
>
>        if (verified_as_different || parent_crc != file_crc)
>         {
> -         std::string msg
> -           = string_printf (_("the debug information found in \"%s\""
> -                              " does not match \"%s\" (CRC mismatch).\n"),
> -                            name.c_str (), objfile_name (parent_objfile));
> +         string_file warnings (true);
> +         warnings.printf (_("the debug information found in \"%ps\""
> +                            " does not match \"%ps\" (CRC mismatch)."),
> +                          styled_string (file_name_style.style (),
> +                                         name.c_str ()),
> +                          styled_string (file_name_style.style (),
> +                                         objfile_name (parent_objfile)));
> +         /* Avoid styling for the debug output.  */
>           if (separate_debug_file_debug)
> -           gdb_printf (gdb_stdlog, "%s", msg.c_str ());
> -         warnings_vector->emplace_back (std::move (msg));
> +           gdb_printf (gdb_stdlog, _("the debug information found in %s"
> +                                     " does not match %s (CRC
> mismatch).\n"),
> +                       name.c_str(), objfile_name (parent_objfile));
> +         warnings_vector->emplace_back (std::move (warnings));
>         }
>
>        return 0;
> @@ -1379,14 +1385,14 @@ show_debug_file_directory (struct ui_file *file,
> int from_tty,
>     string.
>
>     Any warnings generated as part of the lookup process are added to
> -   WARNINGS_VECTOR, one std::string per warning.  */
> +   WARNINGS_VECTOR, one string_file per warning.  */
>
>  static std::string
>  find_separate_debug_file (const char *dir,
>                           const char *canon_dir,
>                           const char *debuglink,
>                           unsigned long crc32, struct objfile *objfile,
> -                         std::vector<std::string> *warnings_vector)
> +                         std::vector<string_file> *warnings_vector)
>  {
>    if (separate_debug_file_debug)
>      gdb_printf (gdb_stdlog,
> @@ -1534,7 +1540,7 @@ terminate_after_last_dir_separator (char *path)
>
>  std::string
>  find_separate_debug_file_by_debuglink
> -  (struct objfile *objfile, std::vector<std::string> *warnings_vector)
> +  (struct objfile *objfile, std::vector<string_file> *warnings_vector)
>  {
>    unsigned long crc32;
>
> diff --git a/gdb/symfile.h b/gdb/symfile.h
> index b433e2be31a..041f6f74f9c 100644
> --- a/gdb/symfile.h
> +++ b/gdb/symfile.h
> @@ -245,10 +245,10 @@ extern void symbol_file_add_separate (const
> gdb_bfd_ref_ptr &, const char *,
>     Returns pathname, or an empty string.
>
>     Any warnings generated as part of this lookup are added to
> -   WARNINGS_VECTOR, one std::string per warning.  */
> +   WARNINGS_VECTOR, one string_file per warning.  */
>
>  extern std::string find_separate_debug_file_by_debuglink
> -  (struct objfile *objfile, std::vector<std::string> *warnings_vector);
> +  (struct objfile *objfile, std::vector<string_file> *warnings_vector);
>
>  /* Build (allocate and populate) a section_addr_info struct from an
>     existing section table.  */
> diff --git a/gdb/ui-file.h b/gdb/ui-file.h
> index de24620e247..4444c8adcdb 100644
> --- a/gdb/ui-file.h
> +++ b/gdb/ui-file.h
> @@ -212,6 +212,13 @@ class string_file : public ui_file
>      return *this;
>    }
>
> +  string_file(string_file &&other)
> +    : m_string (std::move (other.c_str ())),
> +    m_term_out (other.m_term_out)
> +  {
> +
> +  }
> +
>    /* Provide a few convenience methods with the same API as the
>       underlying std::string.  */
>    const char *data () const { return m_string.data (); }
> --
> 2.39.1
>
>