Re: [PATCH] Fix for PR15117

[ali_anwar <ali_anwar@codesourcery.com>]

[-- 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*"] {