From: Andrew Burgess <aburgess@redhat.com>
To: gdb-patches@sourceware.org
Cc: Andrew Burgess <aburgess@redhat.com>,
Guinevere Larsen <guinevere@redhat.com>
Subject: [PATCHv3 1/3] gdb: make get_chars_per_line return an unsigned value
Date: Fri, 5 Dec 2025 19:53:15 +0000 [thread overview]
Message-ID: <eea6f64a87a096c9bb05c3bd9294ec080f4200bf.1764964133.git.aburgess@redhat.com> (raw)
In-Reply-To: <cover.1764964133.git.aburgess@redhat.com>
I noticed that get_chars_per_line currently returns an 'int', but the
value it is returning, from utils.c, is an 'unsigned int'. In some
cases this can cause weird behaviour as an unlimited width terminal
will have UINT_MAX characters per line, which will appear as negative
when returned from get_chars_per_line.
This has been the case since get_chars_per_line was added in commit:
commit 2f2287318b33ddf855a692fcc191f6b25caf4644
Date: Wed Dec 16 18:18:40 2020 +0100
[gdb/cli] Add a progress meter
Lets make get_chars_per_line return an unsigned value, and update all
the uses of this function to hold the result in an unsigned variable.
I ran into this issue when looking at print_gdb_hints (from top.c)
where a very large get_chars_per_line() value would appear negative,
and so the startup hints would be printed without a box when really
they should have been boxed. Someone else noticed this problem while
I was building this patch, and pushed commit:
commit 06e470d8fc0ae0e83fe0977fdf8c011998980891
Date: Sat Nov 29 15:48:55 2025 +0100
gdb: handle unlimited screen width case in print_gdb_hints
This commit works around the signed / unsigned confusion entirely
within print_gdb_hints by adding a case to 'int' in one place. The
change I present here reverts parts of 06e470d8fc0a in favour of
fixing the type of WIDTH within print_gdb_hints.
It is worth nothing that there are other bugs in print_gdb_hints
relating to how WIDTH is handled, but these are fixed in the next
commit.
By updating the return type of get_chars_per_line, I ran into some
issues in cli-out.c relating to how progress bars are handled. The
existing code includes code like:
if (!stream->isatty ()
|| !current_ui->input_interactive_p ()
|| chars_per_line < MIN_CHARS_PER_LINE)
return;
The early return here triggers when progress bars should not be
printed. Notice that when the terminal width is unlimited,
CHARS_PER_LINE will appear negative, and so the early return will
trigger.
It turns out that our testsuite depends on this; the debuginfod tests
don't expect to see a progress bar, and they don't because within the
tests we set the width to unlimited.
By "fixing" the type of CHARS_PER_LINE to 'unsigned int', an unlimited
width terminal no longer triggers the early return, and GDB starts
trying to print a progress bar in our debuginfod tests, which cause
the tests to fail.
I think the real fix is to add a new flag to allow progress bars to be
disabled, the tests can then use this. I will add just such a flag at
the end of this series.
For now though, I propose adding a bodge. If CHARS_PER_LINE is
UINT_MAX, then we should act as if progress bars are disabled. The
above code now becomes:
if (!stream->isatty ()
|| !current_ui->input_interactive_p ()
|| chars_per_line < MIN_CHARS_PER_LINE
|| chars_per_line == UINT_MAX)
return;
This change is in cli_ui_out::clear_progress_notify. There is a
similar change in cli_ui_out::do_progress_notify. With these two
changes, the debuginfod tests are working again. This bodge will be
removed by the last patch in this series.
There's no tests with this commit yet as print_gdb_hints has other
bugs which will be fixed in the next commit. At this point I'll add
some tests.
Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
---
gdb/cli-out.c | 19 ++++++++++---------
gdb/top.c | 10 +++++-----
gdb/utils.c | 3 ++-
gdb/utils.h | 5 +++--
4 files changed, 20 insertions(+), 17 deletions(-)
diff --git a/gdb/cli-out.c b/gdb/cli-out.c
index 27a82f607eb..5aa13a64271 100644
--- a/gdb/cli-out.c
+++ b/gdb/cli-out.c
@@ -298,18 +298,16 @@ cli_ui_out::do_progress_notify (const std::string &msg,
const char *unit,
double howmuch, double total)
{
- int chars_per_line = get_chars_per_line ();
+ unsigned int chars_per_line = get_chars_per_line ();
struct ui_file *stream = get_unbuffered (m_streams.back ());
cli_progress_info &info (m_progress_info.back ());
- if (chars_per_line > MAX_CHARS_PER_LINE)
- chars_per_line = MAX_CHARS_PER_LINE;
-
if (info.state == progress_update::START)
{
if (stream->isatty ()
&& current_ui->input_interactive_p ()
- && chars_per_line >= MIN_CHARS_PER_LINE)
+ && chars_per_line >= MIN_CHARS_PER_LINE
+ && chars_per_line != UINT_MAX)
{
gdb_printf (stream, "%s\n", msg.c_str ());
info.state = progress_update::BAR;
@@ -321,10 +319,12 @@ cli_ui_out::do_progress_notify (const std::string &msg,
}
}
- if (info.state != progress_update::BAR
- || chars_per_line < MIN_CHARS_PER_LINE)
+ if (info.state != progress_update::BAR)
return;
+ if (chars_per_line > MAX_CHARS_PER_LINE)
+ chars_per_line = MAX_CHARS_PER_LINE;
+
if (total > 0 && howmuch >= 0 && howmuch <= 1.0)
{
std::string progress = string_printf (" %3.f%% (%.2f %s)",
@@ -385,14 +385,15 @@ void
cli_ui_out::clear_progress_notify ()
{
struct ui_file *stream = get_unbuffered (m_streams.back ());
- int chars_per_line = get_chars_per_line ();
+ unsigned int chars_per_line = get_chars_per_line ();
scoped_restore save_pagination
= make_scoped_restore (&pagination_enabled, false);
if (!stream->isatty ()
|| !current_ui->input_interactive_p ()
- || chars_per_line < MIN_CHARS_PER_LINE)
+ || chars_per_line < MIN_CHARS_PER_LINE
+ || chars_per_line == UINT_MAX)
return;
if (chars_per_line > MAX_CHARS_PER_LINE)
diff --git a/gdb/top.c b/gdb/top.c
index 1214101ad18..1aefe74b394 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1414,7 +1414,7 @@ box_one_message (ui_file *stream, std::string message, int width)
void
print_gdb_hints (struct ui_file *stream)
{
- int width = get_chars_per_line ();
+ unsigned int width = get_chars_per_line ();
/* Arbitrarily setting maximum width to 80 characters, so that
things are big and visible but not overwhelming. */
@@ -1439,10 +1439,10 @@ print_gdb_hints (struct ui_file *stream)
styled_string (command_style.style (), "apropos <word>"));
/* If there isn't enough space to display the longest URL in a boxed
- style or if screen width is unlimited, use the simple styling of a
- singular visual break. The longest URL is used because the other
- messages may be broken into multiple lines, but URLs can't. */
- if (width - 3 <= (int) docs_url.length ())
+ style, use the simple styling of a singular visual break. The longest
+ URL is used because the other messages may be broken into multiple
+ lines, but URLs can't. */
+ if (width - 3 <= docs_url.length ())
{
for (string_file &msg : styled_msg)
gdb_printf (stream, "%s\n", msg.c_str ());
diff --git a/gdb/utils.c b/gdb/utils.c
index ffe0d639e3a..d322843925b 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1548,9 +1548,10 @@ gdb_flush (struct ui_file *stream)
/* See utils.h. */
-int
+unsigned int
get_chars_per_line ()
{
+ gdb_assert (chars_per_line > 0);
return chars_per_line;
}
diff --git a/gdb/utils.h b/gdb/utils.h
index 0e28f9424e5..6316547653e 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -157,9 +157,10 @@ extern void wrap_here (int);
extern void reinitialize_more_filter (void);
-/* Return the number of characters in a line. */
+/* Return the number of characters in a line. Will never be zero, but can
+ be UINT_MAX, which indicates unlimited characters per line. */
-extern int get_chars_per_line ();
+extern unsigned int get_chars_per_line ();
extern bool pagination_enabled;
--
2.47.1
next prev parent reply other threads:[~2025-12-05 19:54 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-04 16:38 [PATCH 0/4] Fixes and tests related to new boxed hint startup text Andrew Burgess
2025-12-04 16:38 ` [PATCH 1/4] gdb: remove some unnecessary code from print_gdb_hints Andrew Burgess
2025-12-04 17:14 ` Patrick Monnerat
2025-12-04 16:38 ` [PATCH 2/4] gdb: small white space fix in print_gdb_hints Andrew Burgess
2025-12-04 16:38 ` [PATCH 3/4] gdb: make get_chars_per_line return an unsigned value Andrew Burgess
2025-12-04 16:38 ` [PATCH 4/4] gdb: fix crashes and weird output from new boxed hint text Andrew Burgess
2025-12-04 19:02 ` [PATCH 0/4] Fixes and tests related to new boxed hint startup text Guinevere Larsen
2025-12-04 19:32 ` Andrew Burgess
2025-12-04 19:47 ` [PATCH 0/2] " Andrew Burgess
2025-12-04 19:47 ` [PATCH 1/2] gdb: make get_chars_per_line return an unsigned value Andrew Burgess
2025-12-04 19:47 ` [PATCH 2/2] gdb: fix crashes and weird output from new boxed hint text Andrew Burgess
2025-12-05 7:06 ` Eli Zaretskii
2025-12-05 10:17 ` Andrew Burgess
2025-12-05 11:19 ` Eli Zaretskii
2025-12-05 19:53 ` [PATCHv3 0/3] Fixes and tests related to new boxed hint startup text Andrew Burgess
2025-12-05 19:53 ` Andrew Burgess [this message]
2025-12-10 16:48 ` [PATCHv3 1/3] gdb: make get_chars_per_line return an unsigned value Tom Tromey
2025-12-05 19:53 ` [PATCHv3 2/3] gdb: fix crashes and weird output from new boxed hint text Andrew Burgess
2025-12-10 16:51 ` Tom Tromey
2025-12-05 19:53 ` [PATCHv3 3/3] WIP: disable progress bars setting Andrew Burgess
2025-12-06 9:12 ` Eli Zaretskii
2025-12-09 17:32 ` Andrew Burgess
2025-12-10 16:55 ` Tom Tromey
2025-12-15 15:33 ` Andrew Burgess
2025-12-16 7:40 ` Tom de Vries
2025-12-16 10:05 ` Andrew Burgess
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=eea6f64a87a096c9bb05c3bd9294ec080f4200bf.1764964133.git.aburgess@redhat.com \
--to=aburgess@redhat.com \
--cc=gdb-patches@sourceware.org \
--cc=guinevere@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox