* [RFA] More linespec quoting "fixes"
@ 2010-02-24 22:10 Keith Seitz
2010-02-24 22:49 ` Keith Seitz
2010-02-26 23:02 ` Tom Tromey
0 siblings, 2 replies; 6+ messages in thread
From: Keith Seitz @ 2010-02-24 22:10 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 1080 bytes --]
Hi,
Daniel recently committed a patch which fixed a couple of linespec
inconsistencies (allow "file:'function'" and "'file:function'").
I wonder why not also allow "'file':function" and "'file':'function'"?
I couldn't find a good answer as to why these two forms would be
disallowed, so I'm submitting the attached patch to enable these formats.
This also fixes some regressions/conflicts that Daniel's patch had with
my (uncommitted) dwarf2_physname patch which touched linespecs. :-)
No regressions on x86-linux.
Keith
PS. This code must die.(TM)
ChangeLog
2010-02-24 Keith Seitz <keiths@redhat.com>
* linespec.c (decode_line_1): Update comments for is_quote_enclosed.
If the filename portion of the linespec was quoted, recheck the
remainder for additional quoting.
(locate_first_half): Skip over completer chars, too.
testsuite/ChangeLog
2010-02-24 Keith Seitz <keiths@redhat.com>
* gdb.cp/overload.exp: Test that the filename portion of a linespec
can be quoted. Test that both the filename and function/line portions
can be quoted at the same time.
[-- Attachment #2: more-linespec-quoting.patch --]
[-- Type: text/plain, Size: 3596 bytes --]
Index: linespec.c
===================================================================
RCS file: /cvs/src/src/gdb/linespec.c,v
retrieving revision 1.96
diff -u -p -r1.96 linespec.c
--- linespec.c 16 Feb 2010 20:51:49 -0000 1.96
+++ linespec.c 24 Feb 2010 21:39:30 -0000
@@ -694,12 +694,14 @@ decode_line_1 (char **argptr, int funfir
/* This says whether or not something in *ARGPTR is quoted with
completer_quotes (i.e. with single quotes). */
int is_quoted;
- /* Is part of *ARGPTR is enclosed in double quotes? */
+ /* Is *ARGPTR is enclosed in double quotes? */
int is_quote_enclosed;
int is_objc_method = 0;
char *saved_arg = *argptr;
/* If IS_QUOTED, the end of the quoted bit. */
char *end_quote = NULL;
+ /* The "first half" of the linespec. */
+ char *first_half;
if (not_found_ptr)
*not_found_ptr = 0;
@@ -731,7 +733,7 @@ decode_line_1 (char **argptr, int funfir
will point to "". If this is a C++ name, like "A::B::foo", p will
point to "::B::foo". Argptr is not changed by this call. */
- p = locate_first_half (argptr, &is_quote_enclosed);
+ first_half = p = locate_first_half (argptr, &is_quote_enclosed);
/* Check if this is an Objective-C method (anything that starts with
a '+' or '-' and a '['). */
@@ -751,9 +753,6 @@ decode_line_1 (char **argptr, int funfir
return values;
}
- if (is_quoted)
- *argptr = *argptr + 1;
-
/* Does it look like there actually were two parts? */
if (p[0] == ':' || p[0] == '.')
@@ -828,6 +827,17 @@ decode_line_1 (char **argptr, int funfir
/* file_symtab is specified file's symtab, or 0 if no file specified.
arg no longer contains the file name. */
+ /* If the filename was quoted, we must re-check the quotation. */
+
+ if (end_quote == first_half && *end_quote!= '\0')
+ {
+ is_quoted = (**argptr
+ && strchr (get_gdb_completer_quote_characters (),
+ **argptr) != NULL);
+ if (is_quoted)
+ end_quote = skip_quoted (*argptr);
+ }
+
/* Check whether arg is all digits (and sign). */
q = *argptr;
@@ -1055,6 +1065,11 @@ locate_first_half (char **argptr, int *i
(*argptr)++;
p++;
}
+ else if (strchr (get_gdb_completer_quote_characters (), *p))
+ {
+ ++(*argptr);
+ ++p;
+ }
else
*is_quote_enclosed = 0;
for (; *p; p++)
@@ -1574,7 +1589,8 @@ symtab_from_filename (char **argptr, cha
copy = (char *) alloca (p - *argptr + 1);
memcpy (copy, *argptr, p - *argptr);
/* It may have the ending quote right after the file name. */
- if (is_quote_enclosed && copy[p - *argptr - 1] == '"')
+ if ((is_quote_enclosed && copy[p - *argptr - 1] == '"')
+ || copy[p - *argptr - 1] == '\'')
copy[p - *argptr - 1] = 0;
else
copy[p - *argptr] = 0;
Index: testsuite/gdb.cp/overload.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/overload.exp,v
retrieving revision 1.16
diff -u -p -r1.16 overload.exp
--- testsuite/gdb.cp/overload.exp 16 Feb 2010 21:09:32 -0000 1.16
+++ testsuite/gdb.cp/overload.exp 24 Feb 2010 21:39:30 -0000
@@ -299,6 +299,8 @@ gdb_test "list ${srcfile}:intToChar" "in
gdb_test "list ${srcfile}:intToChar(char)" "int intToChar.*"
gdb_test "list ${srcfile}:'intToChar(char)'" "int intToChar.*"
gdb_test "list '${srcfile}:intToChar(char)'" "int intToChar.*"
+gdb_test "list '${srcfile}':intToChar(char)" "int intToChar.*"
+gdb_test "list '${srcfile}':'intToChar(char)'" "int intToChar.*"
# And with filename and namespace... which does not work.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA] More linespec quoting "fixes"
2010-02-24 22:10 [RFA] More linespec quoting "fixes" Keith Seitz
@ 2010-02-24 22:49 ` Keith Seitz
2010-02-26 23:02 ` Tom Tromey
1 sibling, 0 replies; 6+ messages in thread
From: Keith Seitz @ 2010-02-24 22:49 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 751 bytes --]
On 02/24/2010 02:10 PM, Keith Seitz wrote:
> * linespec.c (decode_line_1): Update comments for is_quote_enclosed.
> If the filename portion of the linespec was quoted, recheck the
> remainder for additional quoting.
> (locate_first_half): Skip over completer chars, too.
And, of course, there is a small typo in the patch... locate_first_half
should be:
@@ -1056,7 +1066,14 @@ locate_first_half (char **argptr, int *i
p++;
}
else
- *is_quote_enclosed = 0;
+ {
+ *is_quote_enclosed = 0;
+ if (strchr (get_gdb_completer_quote_characters (), *p))
+ {
+ ++(*argptr);
+ ++p;
+ }
+ }
for (; *p; p++)
{
if (p[0] == '<')
Sorry about that. FWIW, I've attached the corrected patch as well.
Keith
[-- Attachment #2: more-linespec-quoting.patch --]
[-- Type: text/plain, Size: 3639 bytes --]
Index: linespec.c
===================================================================
RCS file: /cvs/src/src/gdb/linespec.c,v
retrieving revision 1.96
diff -u -p -r1.96 linespec.c
--- linespec.c 16 Feb 2010 20:51:49 -0000 1.96
+++ linespec.c 24 Feb 2010 22:44:11 -0000
@@ -694,12 +694,14 @@ decode_line_1 (char **argptr, int funfir
/* This says whether or not something in *ARGPTR is quoted with
completer_quotes (i.e. with single quotes). */
int is_quoted;
- /* Is part of *ARGPTR is enclosed in double quotes? */
+ /* Is *ARGPTR is enclosed in double quotes? */
int is_quote_enclosed;
int is_objc_method = 0;
char *saved_arg = *argptr;
/* If IS_QUOTED, the end of the quoted bit. */
char *end_quote = NULL;
+ /* The "first half" of the linespec. */
+ char *first_half;
if (not_found_ptr)
*not_found_ptr = 0;
@@ -731,7 +733,7 @@ decode_line_1 (char **argptr, int funfir
will point to "". If this is a C++ name, like "A::B::foo", p will
point to "::B::foo". Argptr is not changed by this call. */
- p = locate_first_half (argptr, &is_quote_enclosed);
+ first_half = p = locate_first_half (argptr, &is_quote_enclosed);
/* Check if this is an Objective-C method (anything that starts with
a '+' or '-' and a '['). */
@@ -751,9 +753,6 @@ decode_line_1 (char **argptr, int funfir
return values;
}
- if (is_quoted)
- *argptr = *argptr + 1;
-
/* Does it look like there actually were two parts? */
if (p[0] == ':' || p[0] == '.')
@@ -828,6 +827,17 @@ decode_line_1 (char **argptr, int funfir
/* file_symtab is specified file's symtab, or 0 if no file specified.
arg no longer contains the file name. */
+ /* If the filename was quoted, we must re-check the quotation. */
+
+ if (end_quote == first_half && *end_quote!= '\0')
+ {
+ is_quoted = (**argptr
+ && strchr (get_gdb_completer_quote_characters (),
+ **argptr) != NULL);
+ if (is_quoted)
+ end_quote = skip_quoted (*argptr);
+ }
+
/* Check whether arg is all digits (and sign). */
q = *argptr;
@@ -1056,7 +1066,14 @@ locate_first_half (char **argptr, int *i
p++;
}
else
- *is_quote_enclosed = 0;
+ {
+ *is_quote_enclosed = 0;
+ if (strchr (get_gdb_completer_quote_characters (), *p))
+ {
+ ++(*argptr);
+ ++p;
+ }
+ }
for (; *p; p++)
{
if (p[0] == '<')
@@ -1574,7 +1591,8 @@ symtab_from_filename (char **argptr, cha
copy = (char *) alloca (p - *argptr + 1);
memcpy (copy, *argptr, p - *argptr);
/* It may have the ending quote right after the file name. */
- if (is_quote_enclosed && copy[p - *argptr - 1] == '"')
+ if ((is_quote_enclosed && copy[p - *argptr - 1] == '"')
+ || copy[p - *argptr - 1] == '\'')
copy[p - *argptr - 1] = 0;
else
copy[p - *argptr] = 0;
Index: testsuite/gdb.cp/overload.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/overload.exp,v
retrieving revision 1.16
diff -u -p -r1.16 overload.exp
--- testsuite/gdb.cp/overload.exp 16 Feb 2010 21:09:32 -0000 1.16
+++ testsuite/gdb.cp/overload.exp 24 Feb 2010 22:44:11 -0000
@@ -299,6 +299,8 @@ gdb_test "list ${srcfile}:intToChar" "in
gdb_test "list ${srcfile}:intToChar(char)" "int intToChar.*"
gdb_test "list ${srcfile}:'intToChar(char)'" "int intToChar.*"
gdb_test "list '${srcfile}:intToChar(char)'" "int intToChar.*"
+gdb_test "list '${srcfile}':intToChar(char)" "int intToChar.*"
+gdb_test "list '${srcfile}':'intToChar(char)'" "int intToChar.*"
# And with filename and namespace... which does not work.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA] More linespec quoting "fixes"
2010-02-24 22:10 [RFA] More linespec quoting "fixes" Keith Seitz
2010-02-24 22:49 ` Keith Seitz
@ 2010-02-26 23:02 ` Tom Tromey
2010-03-03 18:00 ` Keith Seitz
1 sibling, 1 reply; 6+ messages in thread
From: Tom Tromey @ 2010-02-26 23:02 UTC (permalink / raw)
To: Keith Seitz; +Cc: gdb-patches
>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:
Keith> Daniel recently committed a patch which fixed a couple of
Keith> linespec inconsistencies (allow "file:'function'" and
Keith> "'file:function'").
Keith> I wonder why not also allow "'file':function" and
Keith> "'file':'function'"?
Keith> I couldn't find a good answer as to why these two forms would be
Keith> disallowed, so I'm submitting the attached patch to enable these
Keith> formats.
The patch seems reasonable enough to me. But, I don't claim to
understand linespec.c, so I'd appreciate it if someone else would take a
look.
My real question is whether we want to do this. We've talked a little
bit about rewriting linespec to be less horrible... will supporting more
syntax make this too hard? I'm open to breaking compatibility a bit --
but I think there is a limit to what we can do there, based on what is
documented.
Tom
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA] More linespec quoting "fixes"
2010-02-26 23:02 ` Tom Tromey
@ 2010-03-03 18:00 ` Keith Seitz
2010-03-04 13:51 ` Daniel Jacobowitz
0 siblings, 1 reply; 6+ messages in thread
From: Keith Seitz @ 2010-03-03 18:00 UTC (permalink / raw)
To: tromey; +Cc: gdb-patches
On 02/26/2010 03:02 PM, Tom Tromey wrote:
> My real question is whether we want to do this. We've talked a little
> bit about rewriting linespec to be less horrible... will supporting more
> syntax make this too hard? I'm open to breaking compatibility a bit --
> but I think there is a limit to what we can do there, based on what is
> documented.
I don't really have an answer for this. This is one of those areas where
the documentation doesn't explain everything in detail.
In an ideal world, I would get rid of quoting entirely (or at least
try), but that's a debate for another time. With this patch, I was
simply attempting to address the issue that single quotes are not
handled in the filename/"first half" of the linespec, and I could find
no reason why they should not. [And if we keep quoting in linespecs, I
cannot think why we would not allow any part of them to be quoted --
consider filenames with spaces (yich).]
In the end, I was attempting to rectify this behavior with CVS HEAD:
(gdb) list 'foo.c':1
No source file named foo.c'.
Keith
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA] More linespec quoting "fixes"
2010-03-03 18:00 ` Keith Seitz
@ 2010-03-04 13:51 ` Daniel Jacobowitz
2010-03-04 18:39 ` Keith Seitz
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Jacobowitz @ 2010-03-04 13:51 UTC (permalink / raw)
To: Keith Seitz; +Cc: tromey, gdb-patches
On Wed, Mar 03, 2010 at 10:00:21AM -0800, Keith Seitz wrote:
> In the end, I was attempting to rectify this behavior with CVS HEAD:
>
> (gdb) list 'foo.c':1
> No source file named foo.c'.
Sounds good to me. There's no use worrying too much about the future
rewrite; we don't know what the code accepts today, let alone what it
will in the future.
--
Daniel Jacobowitz
CodeSourcery
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA] More linespec quoting "fixes"
2010-03-04 13:51 ` Daniel Jacobowitz
@ 2010-03-04 18:39 ` Keith Seitz
0 siblings, 0 replies; 6+ messages in thread
From: Keith Seitz @ 2010-03-04 18:39 UTC (permalink / raw)
To: gdb-patches
On 03/04/2010 05:51 AM, Daniel Jacobowitz wrote:
> Sounds good to me. There's no use worrying too much about the future
> rewrite; we don't know what the code accepts today, let alone what it
> will in the future.
Committed. My thanks to Tom and Daniel for reviewing this.
Keith
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-03-04 18:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-02-24 22:10 [RFA] More linespec quoting "fixes" Keith Seitz
2010-02-24 22:49 ` Keith Seitz
2010-02-26 23:02 ` Tom Tromey
2010-03-03 18:00 ` Keith Seitz
2010-03-04 13:51 ` Daniel Jacobowitz
2010-03-04 18:39 ` Keith Seitz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox