Re: [PATCH 06/11] gdb: Use std::string_view instead of gdb::string_view

[Lancelot SIX <Lancelot.Six@amd.com>]

On 14/10/2023 22:50, Tom Tromey wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
>>>>>> "Lancelot" == Lancelot Six <lancelot.six@amd.com> writes:
> 
> Lancelot> Given that GDB now requires a C++17, replace all uses of
> Lancelot> gdb::string_view with std::string_view.
> 
> Lancelot> This change has mostly been done automatically:
> Lancelot> - gdb::string_view -> std::string_view
> Lancelot> - #include "gdbsupport/gdb_string_view.h" -> #include <string_view>
> 
> Lancelot> The implementation and tests of gdb::string_view are unchanged, they will
> Lancelot> be removed in a following patch.
> 
> IIRC there are some spots using gdb::string_view that construct one with
> nullptr, which isn't allowed by std::string_view.  Maybe a test run
> using the libstdc++ debug mode would detect this?
> 
> Tom

Hi,

It looks like libstdc++'s debug mode won't catch this, but this leads to 
a segfault on my system, so I am fairly confident with the testing.

That being said, I went through the patch again. I think one use can be 
a problem (but is not exercised by the testsuite), and one might be 
worth adding an assertion, just in case.  I have added the following 
locally and am extending the commit message.  It will be in a V2, but 
I'll wait for other comment (if any) before sending it.

Best,
Lancelot.

---
diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index 83218dc4797..78bafec38ca 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -1197,7 +1197,10 @@ tui_getc_1 (FILE *fp)
          Compare keyname instead.  */
        if (ch >= KEY_MAX)
         {
-         auto name = std::string_view (keyname (ch));
+         std::string_view name;
+         if (const char *name_str = keyname (ch);
+             name_str != nullptr)
+           name = std::string_view (name_str);

           /* The following sequences are hardcoded in readline as
              well.  */
diff --git a/gdbsupport/format.h b/gdbsupport/format.h
index 5330bbe361e..26e327a60e8 100644
--- a/gdbsupport/format.h
+++ b/gdbsupport/format.h
@@ -56,6 +56,7 @@ struct format_piece
        argclass (argc),
        n_int_args (n)
    {
+    gdb_assert (str != nullptr);
    }

    bool operator== (const format_piece &other) const