[Bug tui/30389] New: [gdb/tui] status line string longer than necessary

[Bug tui/30389] New: [gdb/tui] status line string longer than necessary

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30389

            Bug ID: 30389
           Summary: [gdb/tui] status line string longer than necessary
           Product: gdb
           Version: 13.1
            Status: NEW
          Severity: normal
          Priority: P2
         Component: tui
          Assignee: unassigned at sourceware dot org
          Reporter: vries at gcc dot gnu.org
  Target Milestone: ---

Consider this bit in tui_locator_window::make_status_line ():
...
  std::string string_val = string.release ();

  if (string.size () < status_size)
    string_val.append (status_size - string.size (), ' ');
  else if (string.size () > status_size)
    string_val.erase (status_size, string.size ());

  return string_val;
...

This looks like it's trying to achieve:
...
  gdb_assert (string_val.size () == status_size);
...
but that's not the case, because string.size is used instead of
string_val.size.

String.size is 0 for me.

Regression since commit 5d10a2041eb ("gdb: add string_file::release method"),
which did:
...
+  std::string string_val = string.release ();
+
   if (string.size () < status_size)
-    string.puts (n_spaces (status_size - string.size ()));
+    string_val.append (status_size - string.size (), ' ');
   else if (string.size () > status_size)
-    string.string ().erase (status_size, string.size ());
+    string_val.erase (status_size, string.size ());

-  return std::move (string.string ());
+  return string_val;
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug tui/30389] [gdb/tui] status line string longer than necessary

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30389

Tom de Vries <vries at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|13.1                        |HEAD
                 CC|                            |simark at simark dot ca,
                   |                            |tromey at sourceware dot org

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug tui/30389] [gdb/tui] status line string longer than necessary

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30389

Tom de Vries <vries at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |minor

--- Comment #1 from Tom de Vries <vries at gcc dot gnu.org> ---
Tentative patch:
...
diff --git a/gdb/tui/tui-stack.c b/gdb/tui/tui-stack.c
index 0a750187c26..4769a7706d1 100644
--- a/gdb/tui/tui-stack.c
+++ b/gdb/tui/tui-stack.c
@@ -183,10 +183,13 @@ tui_locator_window::make_status_line () const

   std::string string_val = string.release ();

-  if (string.size () < status_size)
-    string_val.append (status_size - string.size (), ' ');
-  else if (string.size () > status_size)
-    string_val.erase (status_size, string.size ());
+  size_t len = string_val.size ()
+  if (len < status_size)
+    string_val.append (status_size - len, ' ');
+  else if (len > status_size)
+    string_val.erase (status_size, len);
+
+  gdb_assert (string_val.size () == status_size);

   return string_val;
 }
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug tui/30389] [gdb/tui] status line string longer than necessary

[simon.marchi at polymtl dot ca]

https://sourceware.org/bugzilla/show_bug.cgi?id=30389

Simon Marchi <simon.marchi at polymtl dot ca> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |simon.marchi at polymtl dot ca

--- Comment #2 from Simon Marchi <simon.marchi at polymtl dot ca> ---
(In reply to Tom de Vries from comment #1)
> Tentative patch:
> ...
> diff --git a/gdb/tui/tui-stack.c b/gdb/tui/tui-stack.c
> index 0a750187c26..4769a7706d1 100644
> --- a/gdb/tui/tui-stack.c
> +++ b/gdb/tui/tui-stack.c
> @@ -183,10 +183,13 @@ tui_locator_window::make_status_line () const
>  
>    std::string string_val = string.release ();
>  
> -  if (string.size () < status_size)
> -    string_val.append (status_size - string.size (), ' ');
> -  else if (string.size () > status_size)
> -    string_val.erase (status_size, string.size ());
> +  size_t len = string_val.size ()
> +  if (len < status_size)
> +    string_val.append (status_size - len, ' ');
> +  else if (len > status_size)
> +    string_val.erase (status_size, len);
> +
> +  gdb_assert (string_val.size () == status_size);
>  
>    return string_val;
>  }
> ...

That looks good.  That's what you get when you use meaningless / generic
variable names :).

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug tui/30389] [gdb/tui] status line string longer than necessary

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30389

--- Comment #3 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tom de Vries <vries@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=17f091b31eb2ad8a838bf3ab21201afd2eaf1636

commit 17f091b31eb2ad8a838bf3ab21201afd2eaf1636
Author: Tom de Vries <tdevries@suse.de>
Date:   Wed Apr 26 18:15:56 2023 +0200

    [gdb/tui] Fix length of status line string

    In commit 5d10a2041eb ("gdb: add string_file::release method") this was
added:
    ...
    +  std::string string_val = string.release ();
    ...
    without updating subsequent uses of string.size (), which returns 0 after
the
    string.release () call.

    Fix this by:
    - using string_val.size () instead of string.size (), and
    - adding an assert that would have caught this regression.

    Tested on x86_64-linux.

    Reviewed-By: Simon Marchi <simon.marchi@efficios.com>
    PR tui/30389
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30389

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug tui/30389] [gdb/tui] status line string longer than necessary

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30389

Tom de Vries <vries at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |14.1
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #4 from Tom de Vries <vries at gcc dot gnu.org> ---
Fixed by commit.

-- 
You are receiving this mail because:
You are on the CC list for the bug.