Re: [PATCH] [gdb/tui] Factor out border-mode help text

[Tom de Vries <tdevries@suse.de>]

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

On 6/9/23 15:39, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Tom> OTOH, it changes the translation boundaries, and I'm not sure if the
> Tom> new parts still classify as "entire sentence".
> 
> Tom> Then again, I was not able to find any files containing translations
> Tom> for gdb, so perhaps it doesn't matter.
> 
> Yeah, gdb has a bunch of calls to gettext but nobody has ever gotten
> translations done or installed them in the tree.  I think some of the
> build machinery is missing too... hard to recall, it's been ages since I
> looked at this.
> 
> Tom> +  const std::string help_attribute_mode (_("\
> Tom>     normal          normal display\n\
> ...
> 
> Tom> +  const std::string help_tui_border_mode
> Tom> +    = (std::string ("\
> Tom> +This variable controls the attributes to use for the window borders:\n")
> Tom> +       + help_attribute_mode);
> ...
> 
> The text here isn't passed through gettext.
> Just wrapping the constant string in _() is enough.
>

Fixed.

> Tom> +  add_setshow_enum_cmd ("border-mode", no_class, tui_border_mode_enums,
> Tom> +			&tui_border_mode, _("\
> Tom> +Set the attribute mode to use for the TUI window borders."), _("\
> Tom> +Show the attribute mode to use for the TUI window borders."),
> Tom> +			help_tui_border_mode.c_str (),
> 
> Does add_setshow_enum_cmd copy the string that's passed in?  I wouldn't
> have thought so.  If not, then this can lead to a use-after-free.

It does actually, see add_setshow_cmd_full_erased in cli-decode.c, which 
does:
...
   if (help_doc != NULL)
     {
       full_set_doc = xstrprintf ("%s\n%s", set_doc, help_doc);
       full_show_doc = xstrprintf ("%s\n%s", show_doc, help_doc);
     }
   else
     {
       full_set_doc = make_unique_xstrdup (set_doc);
       full_show_doc = make_unique_xstrdup (show_doc);
     }
...


> Instead help_tui_border_mode would have to be 'static'.
> 
> Tom> +  const std::string help_tui_active_border_mode
> Tom> +    = (std::string ("\
> Tom> +This variable controls the attributes to use for the active window borders:\n")
> Tom> +       + help_attribute_mode);
> 
> Here too.  And similarly with 'static' I think.

This version, rebased on trunk and also addressing the comment from 
Bruno about not needing explicit std::string constructors is what I'm 
planning to commit.

Thanks,
- Tom

[-- Attachment #2: 0001-gdb-tui-Replace-macro-HELP_ATTRIBUTE_MODE-with-std-s.patch --]
[-- Type: text/x-patch, Size: 2813 bytes --]

From f7b7da4c2a7fa1624c21696dcedd96274ad4aba3 Mon Sep 17 00:00:00 2001
From: Tom de Vries <tdevries@suse.de>
Date: Fri, 9 Jun 2023 16:48:22 +0200
Subject: [PATCH] [gdb/tui] Replace macro HELP_ATTRIBUTE_MODE with std::string

Replace macro HELP_ATTRIBUTE_MODE with a std::string.

Tested on x86_64-linux.

Reviewed-By: Bruno Larsen <blarsen@redhat.com>
Reviewed-By: Tom Tromey <tom@tromey.com>
---
 gdb/tui/tui-win.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index 71f961e04d2..7bceebb4525 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -1220,39 +1220,43 @@ This variable controls the border of TUI windows:\n\
 			show_tui_border_kind,
 			&tui_setlist, &tui_showlist);
 
-#define HELP_ATTRIBUTE_MODE "\
+  const std::string help_attribute_mode (_("\
    normal          normal display\n\
    standout        use highlight mode of terminal\n\
    reverse         use reverse video mode\n\
    half            use half bright\n\
    half-standout   use half bright and standout mode\n\
    bold            use extra bright or bold\n\
-   bold-standout   use extra bright or bold with standout mode"
+   bold-standout   use extra bright or bold with standout mode"));
+
+  const std::string help_tui_border_mode
+    = (_("\
+This variable controls the attributes to use for the window borders:\n")
+       + help_attribute_mode);
 
   add_setshow_enum_cmd ("border-mode", no_class, tui_border_mode_enums,
 			&tui_border_mode, _("\
 Set the attribute mode to use for the TUI window borders."), _("\
 Show the attribute mode to use for the TUI window borders."),
-			_("\
-This variable controls the attributes to use for the window borders:\n"
-			  HELP_ATTRIBUTE_MODE),
+			help_tui_border_mode.c_str (),
 			tui_set_var_cmd,
 			show_tui_border_mode,
 			&tui_setlist, &tui_showlist);
 
+  const std::string help_tui_active_border_mode
+    = (_("\
+This variable controls the attributes to use for the active window borders:\n")
+       + help_attribute_mode);
+
   add_setshow_enum_cmd ("active-border-mode", no_class, tui_border_mode_enums,
 			&tui_active_border_mode, _("\
 Set the attribute mode to use for the active TUI window border."), _("\
 Show the attribute mode to use for the active TUI window border."),
-			_("\
-This variable controls the attributes to use for the active window border:\n"
-			  HELP_ATTRIBUTE_MODE),
+			help_tui_active_border_mode.c_str (),
 			tui_set_var_cmd,
 			show_tui_active_border_mode,
 			&tui_setlist, &tui_showlist);
 
-#undef HELP_ATTRIBUTE_MODE
-
   add_setshow_zuinteger_cmd ("tab-width", no_class,
 			     &internal_tab_width, _("\
 Set the tab width, in characters, for the TUI."), _("\

base-commit: a3859ffba3a29b1409714cc2c44e4d0656de3021
-- 
2.35.3