[RFC] init_breakpoint_sal: Add quotes around part of command in error message

[RFC] init_breakpoint_sal: Add quotes around part of command in error message

[Joel Brobecker]

Hello,

What do you guys think of the following change.  If changes:

    (gdb) b *rendez_vous'address TASK 2
    Garbage TASK 2 at end of command

... into ...

    (gdb) b *rendez_vous'address TASK 2
    Garbage `TASK 2' at end of command

(actually, the current output is `Garbage 2 at end of command', but
that's a bug that I am planning on fixing).

Surprisingly, this doesn't appear to be tested at all, at the moment,
as a quick grep for `Garbage' in all of gdb/testsuite did not yield
any match.  I will submit a testcase with my fix that will test this.

gdb/ChangeLog:

        * breakpoint.c (init_breakpoint_sal): Add quotes around part
        of command in two error message.

Untested for now, just trying to get feedback. Will definitely test
before checking in, if people like the change.

Thanks,
-- 
Joel

---
 gdb/breakpoint.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 9eecdf8..9557430 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -8629,7 +8629,7 @@ init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch,
 	  char *arg = b->cond_string;
 	  loc->cond = parse_exp_1 (&arg, block_for_pc (loc->address), 0);
 	  if (*arg)
-              error (_("Garbage %s follows condition"), arg);
+              error (_("Garbage `%s' follows condition"), arg);
 	}
 
       /* Dynamic printf requires and uses additional arguments on the
@@ -8642,7 +8642,7 @@ init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch,
 	    error (_("Format string required"));
 	}
       else if (b->extra_string)
-	error (_("Garbage %s at end of command"), b->extra_string);
+	error (_("Garbage `%s' at end of command"), b->extra_string);
     }
 
   b->display_canonical = display_canonical;
-- 
1.7.1

Re: [RFC] init_breakpoint_sal: Add quotes around part of command in error message

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel>     (gdb) b *rendez_vous'address TASK 2
Joel>     Garbage `TASK 2' at end of command

It makes sense to me.

Tom

Re: [RFC] init_breakpoint_sal: Add quotes around part of command in error message

[Jan Kratochvil]

On Tue, 15 May 2012 16:59:42 +0200, Joel Brobecker wrote:
> -              error (_("Garbage %s follows condition"), arg);
> +              error (_("Garbage `%s' follows condition"), arg);

When it is already discussed I find `%s' to be deprecated in favor of '%s':
	http://www.gnu.org/prep/standards/standards.html#Quote-Characters


Regards,
Jan

Re: [RFC] init_breakpoint_sal: Add quotes around part of command in error message

[Joel Brobecker]

> On Tue, 15 May 2012 16:59:42 +0200, Joel Brobecker wrote:
> > -              error (_("Garbage %s follows condition"), arg);
> > +              error (_("Garbage `%s' follows condition"), arg);
> 
> When it is already discussed I find `%s' to be deprecated in favor of '%s':
> 	http://www.gnu.org/prep/standards/standards.html#Quote-Characters

Ah, yes, that's right. Thanks for catching that. I will replace
the opening quote by another single-quote and commit.

Thanks Tom and Jan.
-- 
Joel

[commit] init_breakpoint_sal: Add quotes around part of command in error message

[Joel Brobecker]

For the record, this is the version I ended up checking in.

gdb/ChangeLog:

        * breakpoint.c (init_breakpoint_sal): Add quotes around part
        of command in two error message.

Tested on x86_64-linux.

---
 gdb/ChangeLog    |    5 +++++
 gdb/breakpoint.c |    4 ++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c488a2f..2af1f59 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2012-05-15  Joel Brobecker  <brobecker@adacore.com>
 
+	* breakpoint.c (init_breakpoint_sal): Add quotes around part
+	of command in two error message.
+
+2012-05-15  Joel Brobecker  <brobecker@adacore.com>
+
 	* breakpoint.c (init_breakpoint_sal): Remove trailing spaces.
 
 2012-05-15  Joel Brobecker  <brobecker@adacore.com>
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 9eecdf8..1a76754 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -8629,7 +8629,7 @@ init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch,
 	  char *arg = b->cond_string;
 	  loc->cond = parse_exp_1 (&arg, block_for_pc (loc->address), 0);
 	  if (*arg)
-              error (_("Garbage %s follows condition"), arg);
+              error (_("Garbage '%s' follows condition"), arg);
 	}
 
       /* Dynamic printf requires and uses additional arguments on the
@@ -8642,7 +8642,7 @@ init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch,
 	    error (_("Format string required"));
 	}
       else if (b->extra_string)
-	error (_("Garbage %s at end of command"), b->extra_string);
+	error (_("Garbage '%s' at end of command"), b->extra_string);
     }
 
   b->display_canonical = display_canonical;
-- 
1.7.1

Re: [RFC] init_breakpoint_sal: Add quotes around part of command in error message

[Stan Shebs]

On 5/15/12 7:59 AM, Joel Brobecker wrote:
> gdb/ChangeLog:
>
>          * breakpoint.c (init_breakpoint_sal): Add quotes around part
>          of command in two error message.
>
> Untested for now, just trying to get feedback. Will definitely test
> before checking in, if people like the change.
>

I think the quoting is a good general principle to follow whenever 
reporting any string that originates from user input, or from the 
target; cut-n-paste errors and twitchy mice can conspire to generate 
thoroughly confusing errors if the strings are not delimited somehow.  I 
could go with either double quotes or single quotes, but as Jan points 
out, the general style seems to prefer a pair of single quotes.

Stan