Patchwork "enable count" user input error handling (PR gdb/15678)

login
register
mail settings
Submitter Simon Marchi
Date Feb. 6, 2015, 3:31 p.m.
Message ID <54D4DE6C.9050709@ericsson.com>
Download mbox | patch
Permalink /patch/4940/
State New
Headers show

Comments

Simon Marchi - Feb. 6, 2015, 3:31 p.m.
On 13-06-26 03:15 PM, Pedro Alves wrote:
> On 06/26/2013 07:33 PM, Simon Marchi wrote:
>> Typing "enable count" by itself crashes GDB. Also, if you omit the
>> breakpoint number/range, the error message is not very clear:
>>
>> (gdb) enable count 2
>> warning: bad breakpoint number at or near ''
>> (gdb) enable count
>> Segmentation fault (core dumped)
>>
>> With this patch, the error messages are slightly more helpful:
>>
>> (gdb) enable count 2
>> Argument required (one or more breakpoint numbers).
> 
> Most if not all enable/disable breakpoint-and-similars commands
> when not passed a breakpoint number apply to all breakpoints.
> Supporting that would make it possible to disable the count
> for all breakpoints with just "enable count 0".
> 
> WDY(all)T of making that work?
> 
>> (gdb) enable count
>> Argument required (hit count).
>>
>> They are not as helpful to the user as I would like, but it's better
>> than crashing. Suggestions are welcome.
>>
>> Simon
>>
>> gdb/ChangeLog:
>> 2013-06-26  Simon Marchi  <simon.marchi@ericsson.com>
>>
>> 	* breakpoint.c (map_breakpoint_numbers): Check for empty args
>> 	string.
>> 	(enable_count_command): Check args for NULL value.
>>
>> gdb/testsuite/ChangeLog:
>> 2013-06-26  Simon Marchi  <simon.marchi@ericsson.com>
>>
>> 	* gdb.base/ena-dis-br.exp: Test "enable count" for bad user input.
> 
> This is OK.  Please add a line to the ChangeLog entries mentioning
> the PR number.  That'll make the commit automatically be logged with
> the PR (provided the ChangeLog entry is pasted in the commit log).
> See existing entries for examples, or the contribution
> checklist page in the wiki for a more format explanation.
> 
> Thanks!

While scanning the bugzilla, I realized that this patch was never pushed. I
submitted it before I had my copyright assignment and then forgot about it.

So I added the PR number in the changelog and pushed it like that, since it's
not very critical.

From b9d6130764916fac3d9bcfde2d672053a0ef3316 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Mon, 2 Feb 2015 14:57:31 -0500
Subject: [PATCH] "enable count" user input error handling (PR gdb/15678)

Typing "enable count" by itself crashes GDB. Also, if you omit the
breakpoint number/range, the error message is not very clear:

(gdb) enable count 2
warning: bad breakpoint number at or near ''
(gdb) enable count
Segmentation fault (core dumped)

With this patch, the error messages are slightly more helpful:

(gdb) enable count 2
Argument required (one or more breakpoint numbers).
(gdb) enable count
Argument required (hit count).

gdb/ChangeLog:

	PR gdb/15678
	* breakpoint.c (map_breakpoint_numbers): Check for empty args
	string.
	(enable_count_command): Check args for NULL value.

gdb/testsuite/ChangeLog:

	PR gdb/15678
	* gdb.base/ena-dis-br.exp: Test "enable count" for bad user input.
---
 gdb/ChangeLog                         | 6 ++++++
 gdb/breakpoint.c                      | 9 +++++++--
 gdb/testsuite/ChangeLog               | 5 +++++
 gdb/testsuite/gdb.base/ena-dis-br.exp | 8 ++++++++
 4 files changed, 26 insertions(+), 2 deletions(-)

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7c7e878..dbeef19 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@ 
+2015-02-06  Simon Marchi  <simon.marchi@ericsson.com>
+
+	PR gdb/15678
+	* breakpoint.c (map_breakpoint_numbers): Check for empty args string.
+	(enable_count_command): Check args for NULL value.
+
 2015-02-05  Doug Evans  <xdje42@gmail.com>

 	* guile/scm-frame.c: Fix spelling errors in a comment.
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index af0c2cf..2804453 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -14880,7 +14880,7 @@  map_breakpoint_numbers (char *args, void (*function) (struct breakpoint *,
   int match;
   struct get_number_or_range_state state;

-  if (args == 0)
+  if (args == 0 || *args == '\0')
     error_no_arg (_("one or more breakpoint numbers"));

   init_number_or_range (&state, args);
@@ -15217,7 +15217,12 @@  do_map_enable_count_breakpoint (struct breakpoint *bpt, void *countptr)
 static void
 enable_count_command (char *args, int from_tty)
 {
-  int count = get_number (&args);
+  int count;
+
+  if (args == NULL)
+    error_no_arg (_("hit count"));
+
+  count = get_number (&args);

   map_breakpoint_numbers (args, do_map_enable_count_breakpoint, &count);
 }
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index f8ca4e8..c458bd7 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@ 
+2015-02-06  Simon Marchi  <simon.marchi@ericsson.com>
+
+	PR gdb/15678
+	* gdb.base/ena-dis-br.exp: Test "enable count" for bad user input.
+
 2015-02-06  Pedro Alves  <palves@redhat.com>

 	* gdb.threads/attach-many-short-lived-threads.c (SECONDS): New
diff --git a/gdb/testsuite/gdb.base/ena-dis-br.exp b/gdb/testsuite/gdb.base/ena-dis-br.exp
index adb4001..84f84cae 100644
--- a/gdb/testsuite/gdb.base/ena-dis-br.exp
+++ b/gdb/testsuite/gdb.base/ena-dis-br.exp
@@ -151,6 +151,14 @@  set bp [break_at $bp_location7 "line $bp_location7"]

 set bp2 [break_at marker1 " line $bp_location15"]

+gdb_test "enable count" \
+    "Argument required \\(hit count\\)\\." \
+    "enable count missing arguments"
+
+gdb_test "enable count 2" \
+    "Argument required \\(one or more breakpoint numbers\\)\\." \
+    "enable count missing breakpoint number"
+
 gdb_test_no_output "enable count 2 $bp" "disable break with count"

 gdb_test "continue" \