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: 10644 bytes --]

Ping2

On Thu, Mar 30, 2023 at 10:11 AM Alexandra Petlanova Hajkova <
ahajkova@redhat.com> wrote:

> 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
>>
>>