diff mbox series

gdb: do not handle a NULL linebuffer in pager_file::puts

Message ID 20250305162817.63795-1-simon.marchi@efficios.com
State New
Headers
Series gdb: do not handle a NULL linebuffer in pager_file::puts |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 fail Patch failed to apply
linaro-tcwg-bot/tcwg_gdb_build--master-arm fail Patch failed to apply

Commit Message

Simon Marchi March 5, 2025, 4:28 p.m. UTC 
  From: Simon Marchi <simon.marchi@polymtl.ca>

This patch [1] has shown that different implementations of ui_file::puts
handle a NULL line differently.  pager_file::puts handles a NULL
argument gracefully, as a no-op, while other implementations don't and
likely crash.  This causes subtle bugs: things will be working until the
current ui_file is suddenly not a pager_file anymore.  I think it would
be better to be consistent here, so change pager_file::puts to not
accept a NULL line.

A regular test run on Linux shows no regression.

[1] https://inbox.sourceware.org/gdb-patches/edfe6e17-1c20-4a4c-944f-247ff71b6c10@simark.ca/T/#m864aea10de8ca6fa84757971fcbaf3180e2eaefa

Change-Id: Ieb465c86cd2c42a248cf481cd174c8622ef6724b
---
 gdb/utils.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)


base-commit: e283a286894dae2af87a66c1fc2620dfce27d93d

Comments

Tom Tromey March 5, 2025, 5:14 p.m. UTC | #1 
>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:

Simon> From: Simon Marchi <simon.marchi@polymtl.ca>
Simon> This patch [1] has shown that different implementations of ui_file::puts
Simon> handle a NULL line differently.  pager_file::puts handles a NULL
Simon> argument gracefully, as a no-op, while other implementations don't and
Simon> likely crash.  This causes subtle bugs: things will be working until the
Simon> current ui_file is suddenly not a pager_file anymore.  I think it would
Simon> be better to be consistent here, so change pager_file::puts to not
Simon> accept a NULL line.

Ok.
Approved-By: Tom Tromey <tom@tromey.com>

Tom
Simon Marchi March 5, 2025, 5:19 p.m. UTC | #2 
On 3/5/25 12:14 PM, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:
> 
> Simon> From: Simon Marchi <simon.marchi@polymtl.ca>
> Simon> This patch [1] has shown that different implementations of ui_file::puts
> Simon> handle a NULL line differently.  pager_file::puts handles a NULL
> Simon> argument gracefully, as a no-op, while other implementations don't and
> Simon> likely crash.  This causes subtle bugs: things will be working until the
> Simon> current ui_file is suddenly not a pager_file anymore.  I think it would
> Simon> be better to be consistent here, so change pager_file::puts to not
> Simon> accept a NULL line.
> 
> Ok.
> Approved-By: Tom Tromey <tom@tromey.com>
> 
> Tom

Thanks, pushed.

Simon
diff mbox series

Patch

diff --git a/gdb/utils.c b/gdb/utils.c
index 4027d4f26c38..9842bfc8be0f 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1638,8 +1638,7 @@  pager_file::puts (const char *linebuffer)
 {
   const char *lineptr;
 
-  if (linebuffer == 0)
-    return;
+  gdb_assert (linebuffer != nullptr);
 
   /* Don't do any filtering or wrapping if both are disabled.  */
   if (batch_flag