Re: [PATCH] Fix for PR15117

ali_anwar
2013-08-07T05:42:44Z

[-- Attachment #1: Type: text/plain, Size: 1005 bytes --]

On 08/02/2013 10:49 PM, Keith Seitz wrote:
>
> Two tiny nits:
>
>> + {
>> + /* Not able to parse user specified variable. Do not
>> + throw an error here. parse_linespec will do it for us*/
>
> This comment is not formatted properly: two spaces after '.'. Use
> complete sentences (where possible/feasible). This should probably read:
>
> /* The user-specified variable was not valid. Do not
> throw an error here. parse_linespec will do it for us. */
>
> [i.e, just copy the bits from the following block]

Fixed.

>> +# Test break via convenience variable with file name
>> +#
>> +set line [gdb_get_line_number "set breakpoint 1 here"]
>> +gdb_test_no_output "set \$l = $line"
>
> I'm showing extra whitespace at the end of the above line. Could you
> double-check that before committing?
>

Fixed.

> With those trivial things fixed, I think this patch is ready for a
> global maintainer to review and approve.
>

Thank you for reviewing the patch again.

Regards,
-Ali

[-- Attachment #2: PR15117_v3.patch --]
[-- Type: text/x-patch, Size: 2227 bytes --]

Index: linespec.c
===================================================================
RCS file: /cvs/src/src/gdb/linespec.c,v
retrieving revision 1.185
diff -u -r1.185 linespec.c
--- linespec.c 30 May 2013 16:57:38 -0000 1.185
+++ linespec.c 7 Aug 2013 05:32:04 -0000
@@ -1649,7 +1649,7 @@
else
{
/* NAME was not a function or a method. So it must be a label
- name. */
+ name or user specified variable like "break foo.c:$zippo". */
labels = find_label_symbols (PARSER_STATE (parser), NULL,
&symbols, name);
if (labels != NULL)
@@ -1660,6 +1660,22 @@
symbols = NULL;
discard_cleanups (cleanup);
}
+ else if (token.type == LSTOKEN_STRING
+ && *LS_TOKEN_STOKEN (token).ptr == '$')
+ {
+ /* User specified a convenience variable or history value. */
+ PARSER_RESULT (parser)->line_offset
+ = linespec_parse_variable (PARSER_STATE (parser), name);
+
+ if (PARSER_RESULT (parser)->line_offset.sign == LINE_OFFSET_UNKNOWN)
+ {
+ /* The user-specified variable was not valid. Do not
+ throw an error here. parse_linespec will do it for us. */
+ PARSER_RESULT (parser)->function_name = name;
+ discard_cleanups (cleanup);
+ return;
+ }
+ }
else
{
/* The name is also not a label. Abort parsing. Do not throw
Index: testsuite/gdb.base/break.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/break.exp,v
retrieving revision 1.58
diff -u -r1.58 break.exp
--- testsuite/gdb.base/break.exp 7 Jun 2013 17:31:07 -0000 1.58
+++ testsuite/gdb.base/break.exp 7 Aug 2013 05:40:12 -0000
@@ -957,6 +957,18 @@
}
}

+#
+# Test break via convenience variable with file name
+#
+set line [gdb_get_line_number "set breakpoint 1 here"]
+gdb_test_no_output "set \$l = $line"
+gdb_breakpoint ${srcfile}:\$l
+
+gdb_test_no_output "set \$foo=81.5" \
+ "set convenience variable \$foo to 81.5"
+gdb_test "break $srcfile:\$foo" \
+ "Convenience variables used in line specs must have integer values.*" \
+ "set breakpoint via non-integer convenience variable disallowed"

# Reset the default arguments for VxWorks
if [istarget "*-*-vxworks*"] {