* Re: [RFA] Fix mi-break.exp: 'b "basics.c":16'
@ 2002-04-03 7:58 Michael Elizabeth Chastain
2002-04-03 8:20 ` Daniel Jacobowitz
0 siblings, 1 reply; 11+ messages in thread
From: Michael Elizabeth Chastain @ 2002-04-03 7:58 UTC (permalink / raw)
To: drow, gdb-patches
I have a question about this linespec patch.
After I apply the patch, decode_line_1 looks like this:
/* Extract the file name. */
p1 = p;
while (p != *argptr && p[-1] == ' ')
--p;
if ((*p == '"') && is_quote_enclosed)
--p;
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] == '"')
copy[p - *argptr - 1] = 0;
else
copy[p - *argptr] = 0;
If there is a '"' at the end, then it won't be copied into "copy".
So these lines look redundant to me:
if (is_quote_enclosed && copy[p - *argptr - 1] == '"')
copy[p - *argptr - 1] = 0;
Or maybe I am missing something, such as nested quotes?
Michael C
2002-04-02 Daniel Jacobowitz <drow@mvista.com>
* linespec.c (decode_line_1): Check for a double quote after
a filename correctly.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [RFA] Fix mi-break.exp: 'b "basics.c":16'
2002-04-03 7:58 [RFA] Fix mi-break.exp: 'b "basics.c":16' Michael Elizabeth Chastain
@ 2002-04-03 8:20 ` Daniel Jacobowitz
0 siblings, 0 replies; 11+ messages in thread
From: Daniel Jacobowitz @ 2002-04-03 8:20 UTC (permalink / raw)
To: Michael Elizabeth Chastain; +Cc: gdb-patches
On Wed, Apr 03, 2002 at 09:58:24AM -0600, Michael Elizabeth Chastain wrote:
> I have a question about this linespec patch.
>
> After I apply the patch, decode_line_1 looks like this:
>
> /* Extract the file name. */
> p1 = p;
> while (p != *argptr && p[-1] == ' ')
> --p;
> if ((*p == '"') && is_quote_enclosed)
> --p;
> 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] == '"')
> copy[p - *argptr - 1] = 0;
> else
> copy[p - *argptr] = 0;
>
> If there is a '"' at the end, then it won't be copied into "copy".
> So these lines look redundant to me:
>
> if (is_quote_enclosed && copy[p - *argptr - 1] == '"')
> copy[p - *argptr - 1] = 0;
>
> Or maybe I am missing something, such as nested quotes?
The two lines:
if ((*p == '"') && is_quote_enclosed)
--p;
I left because they were there, and I don't understand every situation
that can reach this code. But for "basics.c":16, *p == ':'. So the
quote is actually copied.
>
> Michael C
>
> 2002-04-02 Daniel Jacobowitz <drow@mvista.com>
>
> * linespec.c (decode_line_1): Check for a double quote after
> a filename correctly.
>
>
--
Daniel Jacobowitz Carnegie Mellon University
MontaVista Software Debian GNU/Linux Developer
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFA] Fix mi-break.exp: 'b "basics.c":16'
@ 2002-04-03 9:39 Michael Elizabeth Chastain
0 siblings, 0 replies; 11+ messages in thread
From: Michael Elizabeth Chastain @ 2002-04-03 9:39 UTC (permalink / raw)
To: drow; +Cc: gdb-patches
I get 10 for 10 on my native i686-pc-linux-gnu test bed.
I recommend this patch for approval.
Michael C
===
2002-04-02 Daniel Jacobowitz <drow@mvista.com>
* linespec.c (decode_line_1): Check for a double quote after
a filename correctly.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFA] Fix mi-break.exp: 'b "basics.c":16'
@ 2002-04-03 8:39 Michael Elizabeth Chastain
0 siblings, 0 replies; 11+ messages in thread
From: Michael Elizabeth Chastain @ 2002-04-03 8:39 UTC (permalink / raw)
To: drow; +Cc: gdb-patches
Daniel Jacobowitz writes:
> But for "basics.c":16, *p == ':'. So the quote is actually copied.
Okay, I understand now. *p points to the delimiter, not to the '"'.
The earlier lines:
if ((*p == '"') && is_quote_enclosed)
--p;
are actually the anomalous lines.
What a mess. I'm glad that your patch is removing net lines.
Maybe someday somebody can rewrite this mess.
My preliminary results on one configuration are the same as yours:
fixes four FAILs in gdb.mi, no regressions. My test bed is about
50% done. Results in an hour or so.
Michael C
^ permalink raw reply [flat|nested] 11+ messages in thread* [RFA] Fix mi-break.exp: 'b "basics.c":16'
@ 2002-04-02 16:29 Daniel Jacobowitz
2002-05-09 15:31 ` Daniel Jacobowitz
2002-05-09 15:53 ` Michael Snyder
0 siblings, 2 replies; 11+ messages in thread
From: Daniel Jacobowitz @ 2002-04-02 16:29 UTC (permalink / raw)
To: gdb-patches
[I dislike decode_line_1. But that seems to be the general consensus...]
There was a logic error in the code to handle "file":line. Several,
actually. I suspect that it once worked and had bitrotten as the behavior
of the function changed. For instance, at the beginning of the function:
if (p[0] == '"')
{
is_quote_enclosed = 1;
(*argptr)++;
p++;
}
Then below it checked 'is_quote_enclosed && (**argptr) == '"'). That'll
only be true given a literal '"":<line>', which was not the intent of the
test.
This patch updates the behavior, should change nothing else, and causes no
regressions. OK to check in?
--
Daniel Jacobowitz Carnegie Mellon University
MontaVista Software Debian GNU/Linux Developer
2002-04-02 Daniel Jacobowitz <drow@mvista.com>
* linespec.c (decode_line_1): Check for a double quote after
a filename correctly.
Index: linespec.c
===================================================================
RCS file: /cvs/src/src/gdb/linespec.c,v
retrieving revision 1.17
diff -u -p -r1.17 linespec.c
--- linespec.c 2002/03/22 18:57:07 1.17
+++ linespec.c 2002/04/03 00:19:30
@@ -929,20 +929,12 @@ decode_line_1 (char **argptr, int funfir
if ((*p == '"') && is_quote_enclosed)
--p;
copy = (char *) alloca (p - *argptr + 1);
- if ((**argptr == '"') && is_quote_enclosed)
- {
- memcpy (copy, *argptr + 1, p - *argptr - 1);
- /* It may have the ending quote right after the file name */
- if (copy[p - *argptr - 2] == '"')
- copy[p - *argptr - 2] = 0;
- else
- copy[p - *argptr - 1] = 0;
- }
+ memcpy (copy, *argptr, p - *argptr);
+ /* It may have the ending quote right after the file name */
+ if (is_quote_enclosed && copy[p - *argptr - 1] == '"')
+ copy[p - *argptr - 1] = 0;
else
- {
- memcpy (copy, *argptr, p - *argptr);
- copy[p - *argptr] = 0;
- }
+ copy[p - *argptr] = 0;
/* Find that file's data. */
s = lookup_symtab (copy);
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [RFA] Fix mi-break.exp: 'b "basics.c":16'
2002-04-02 16:29 Daniel Jacobowitz
@ 2002-05-09 15:31 ` Daniel Jacobowitz
2002-05-09 15:35 ` Elena Zannoni
2002-05-09 15:53 ` Michael Snyder
1 sibling, 1 reply; 11+ messages in thread
From: Daniel Jacobowitz @ 2002-05-09 15:31 UTC (permalink / raw)
To: gdb-patches
On Tue, Apr 02, 2002 at 07:29:59PM -0500, Daniel Jacobowitz wrote:
> [I dislike decode_line_1. But that seems to be the general consensus...]
>
> There was a logic error in the code to handle "file":line. Several,
> actually. I suspect that it once worked and had bitrotten as the behavior
> of the function changed. For instance, at the beginning of the function:
>
> if (p[0] == '"')
> {
> is_quote_enclosed = 1;
> (*argptr)++;
> p++;
> }
>
> Then below it checked 'is_quote_enclosed && (**argptr) == '"'). That'll
> only be true given a literal '"":<line>', which was not the intent of the
> test.
>
>
> This patch updates the behavior, should change nothing else, and causes no
> regressions. OK to check in?
Only Michael Chastain (who recommended it for approval) had anything to
say about this patch. Ping?
> 2002-04-02 Daniel Jacobowitz <drow@mvista.com>
>
> * linespec.c (decode_line_1): Check for a double quote after
> a filename correctly.
>
> Index: linespec.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/linespec.c,v
> retrieving revision 1.17
> diff -u -p -r1.17 linespec.c
> --- linespec.c 2002/03/22 18:57:07 1.17
> +++ linespec.c 2002/04/03 00:19:30
> @@ -929,20 +929,12 @@ decode_line_1 (char **argptr, int funfir
> if ((*p == '"') && is_quote_enclosed)
> --p;
> copy = (char *) alloca (p - *argptr + 1);
> - if ((**argptr == '"') && is_quote_enclosed)
> - {
> - memcpy (copy, *argptr + 1, p - *argptr - 1);
> - /* It may have the ending quote right after the file name */
> - if (copy[p - *argptr - 2] == '"')
> - copy[p - *argptr - 2] = 0;
> - else
> - copy[p - *argptr - 1] = 0;
> - }
> + memcpy (copy, *argptr, p - *argptr);
> + /* It may have the ending quote right after the file name */
> + if (is_quote_enclosed && copy[p - *argptr - 1] == '"')
> + copy[p - *argptr - 1] = 0;
> else
> - {
> - memcpy (copy, *argptr, p - *argptr);
> - copy[p - *argptr] = 0;
> - }
> + copy[p - *argptr] = 0;
>
> /* Find that file's data. */
> s = lookup_symtab (copy);
>
--
Daniel Jacobowitz Carnegie Mellon University
MontaVista Software Debian GNU/Linux Developer
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [RFA] Fix mi-break.exp: 'b "basics.c":16'
2002-05-09 15:31 ` Daniel Jacobowitz
@ 2002-05-09 15:35 ` Elena Zannoni
0 siblings, 0 replies; 11+ messages in thread
From: Elena Zannoni @ 2002-05-09 15:35 UTC (permalink / raw)
To: Daniel Jacobowitz; +Cc: gdb-patches
Daniel Jacobowitz writes:
> On Tue, Apr 02, 2002 at 07:29:59PM -0500, Daniel Jacobowitz wrote:
> > [I dislike decode_line_1. But that seems to be the general consensus...]
> >
> > There was a logic error in the code to handle "file":line. Several,
> > actually. I suspect that it once worked and had bitrotten as the behavior
> > of the function changed. For instance, at the beginning of the function:
> >
> > if (p[0] == '"')
> > {
> > is_quote_enclosed = 1;
> > (*argptr)++;
> > p++;
> > }
> >
> > Then below it checked 'is_quote_enclosed && (**argptr) == '"'). That'll
> > only be true given a literal '"":<line>', which was not the intent of the
> > test.
> >
> >
> > This patch updates the behavior, should change nothing else, and causes no
> > regressions. OK to check in?
>
> Only Michael Chastain (who recommended it for approval) had anything to
> say about this patch. Ping?
>
Argh. I thought from the subject it was a testsuite patch, not a
linespec patch. I'll take a look. Just a sec.
Elena
>
> > 2002-04-02 Daniel Jacobowitz <drow@mvista.com>
> >
> > * linespec.c (decode_line_1): Check for a double quote after
> > a filename correctly.
> >
> > Index: linespec.c
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/linespec.c,v
> > retrieving revision 1.17
> > diff -u -p -r1.17 linespec.c
> > --- linespec.c 2002/03/22 18:57:07 1.17
> > +++ linespec.c 2002/04/03 00:19:30
> > @@ -929,20 +929,12 @@ decode_line_1 (char **argptr, int funfir
> > if ((*p == '"') && is_quote_enclosed)
> > --p;
> > copy = (char *) alloca (p - *argptr + 1);
> > - if ((**argptr == '"') && is_quote_enclosed)
> > - {
> > - memcpy (copy, *argptr + 1, p - *argptr - 1);
> > - /* It may have the ending quote right after the file name */
> > - if (copy[p - *argptr - 2] == '"')
> > - copy[p - *argptr - 2] = 0;
> > - else
> > - copy[p - *argptr - 1] = 0;
> > - }
> > + memcpy (copy, *argptr, p - *argptr);
> > + /* It may have the ending quote right after the file name */
> > + if (is_quote_enclosed && copy[p - *argptr - 1] == '"')
> > + copy[p - *argptr - 1] = 0;
> > else
> > - {
> > - memcpy (copy, *argptr, p - *argptr);
> > - copy[p - *argptr] = 0;
> > - }
> > + copy[p - *argptr] = 0;
> >
> > /* Find that file's data. */
> > s = lookup_symtab (copy);
> >
>
> --
> Daniel Jacobowitz Carnegie Mellon University
> MontaVista Software Debian GNU/Linux Developer
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFA] Fix mi-break.exp: 'b "basics.c":16'
2002-04-02 16:29 Daniel Jacobowitz
2002-05-09 15:31 ` Daniel Jacobowitz
@ 2002-05-09 15:53 ` Michael Snyder
2002-05-09 16:05 ` Daniel Jacobowitz
1 sibling, 1 reply; 11+ messages in thread
From: Michael Snyder @ 2002-05-09 15:53 UTC (permalink / raw)
To: Daniel Jacobowitz; +Cc: gdb-patches
Daniel Jacobowitz wrote:
>
> [I dislike decode_line_1. But that seems to be the general consensus...]
>
> There was a logic error in the code to handle "file":line. Several,
> actually. I suspect that it once worked and had bitrotten as the behavior
> of the function changed. For instance, at the beginning of the function:
>
> if (p[0] == '"')
> {
> is_quote_enclosed = 1;
> (*argptr)++;
> p++;
> }
>
> Then below it checked 'is_quote_enclosed && (**argptr) == '"').
[line break inserted]
> That'll only be true given a literal '"":<line>', which was not
> the intent of the test.
I don't understand this statement. Between the two bits of text
you've named, both p and *argptr are changed. Seems to me, there
could be any number of characters between the two quotes.
> This patch updates the behavior, should change nothing else, and causes no
> regressions. OK to check in?
>
> --
> Daniel Jacobowitz Carnegie Mellon University
> MontaVista Software Debian GNU/Linux Developer
>
> 2002-04-02 Daniel Jacobowitz <drow@mvista.com>
>
> * linespec.c (decode_line_1): Check for a double quote after
> a filename correctly.
>
> Index: linespec.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/linespec.c,v
> retrieving revision 1.17
> diff -u -p -r1.17 linespec.c
> --- linespec.c 2002/03/22 18:57:07 1.17
> +++ linespec.c 2002/04/03 00:19:30
> @@ -929,20 +929,12 @@ decode_line_1 (char **argptr, int funfir
> if ((*p == '"') && is_quote_enclosed)
> --p;
> copy = (char *) alloca (p - *argptr + 1);
> - if ((**argptr == '"') && is_quote_enclosed)
> - {
> - memcpy (copy, *argptr + 1, p - *argptr - 1);
> - /* It may have the ending quote right after the file name */
> - if (copy[p - *argptr - 2] == '"')
> - copy[p - *argptr - 2] = 0;
> - else
> - copy[p - *argptr - 1] = 0;
> - }
> + memcpy (copy, *argptr, p - *argptr);
> + /* It may have the ending quote right after the file name */
> + if (is_quote_enclosed && copy[p - *argptr - 1] == '"')
> + copy[p - *argptr - 1] = 0;
> else
> - {
> - memcpy (copy, *argptr, p - *argptr);
> - copy[p - *argptr] = 0;
> - }
> + copy[p - *argptr] = 0;
>
> /* Find that file's data. */
> s = lookup_symtab (copy);
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [RFA] Fix mi-break.exp: 'b "basics.c":16'
2002-05-09 15:53 ` Michael Snyder
@ 2002-05-09 16:05 ` Daniel Jacobowitz
2002-05-09 16:32 ` Elena Zannoni
0 siblings, 1 reply; 11+ messages in thread
From: Daniel Jacobowitz @ 2002-05-09 16:05 UTC (permalink / raw)
To: Michael Snyder; +Cc: gdb-patches
On Thu, May 09, 2002 at 03:40:42PM -0700, Michael Snyder wrote:
> Daniel Jacobowitz wrote:
> >
> > [I dislike decode_line_1. But that seems to be the general consensus...]
> >
> > There was a logic error in the code to handle "file":line. Several,
> > actually. I suspect that it once worked and had bitrotten as the behavior
> > of the function changed. For instance, at the beginning of the function:
> >
> > if (p[0] == '"')
> > {
> > is_quote_enclosed = 1;
> > (*argptr)++;
> > p++;
> > }
> >
> > Then below it checked 'is_quote_enclosed && (**argptr) == '"').
>
> [line break inserted]
>
> > That'll only be true given a literal '"":<line>', which was not
> > the intent of the test.
>
> I don't understand this statement. Between the two bits of text
> you've named, both p and *argptr are changed. Seems to me, there
> could be any number of characters between the two quotes.
This is the part that took me the longest to work out in the first
place, I think. `p' being changed doesn't matter here; it should end
up point at (before?) the colon.
The first if statement is at line 630. The second is at 929. Between
the two, *argptr is only changed if we found a C++ class. It might be
changed on "interestingly" named files, also, but if so it's purely a
bug in the twistiness of linespec. At line 633 it points to the
_second_ character (right after the first quote), so if **argptr ==
'"', that means the second quote is right after the first.
>
>
> > This patch updates the behavior, should change nothing else, and causes no
> > regressions. OK to check in?
> >
> > --
> > Daniel Jacobowitz Carnegie Mellon University
> > MontaVista Software Debian GNU/Linux Developer
> >
> > 2002-04-02 Daniel Jacobowitz <drow@mvista.com>
> >
> > * linespec.c (decode_line_1): Check for a double quote after
> > a filename correctly.
> >
> > Index: linespec.c
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/linespec.c,v
> > retrieving revision 1.17
> > diff -u -p -r1.17 linespec.c
> > --- linespec.c 2002/03/22 18:57:07 1.17
> > +++ linespec.c 2002/04/03 00:19:30
> > @@ -929,20 +929,12 @@ decode_line_1 (char **argptr, int funfir
> > if ((*p == '"') && is_quote_enclosed)
> > --p;
> > copy = (char *) alloca (p - *argptr + 1);
> > - if ((**argptr == '"') && is_quote_enclosed)
> > - {
> > - memcpy (copy, *argptr + 1, p - *argptr - 1);
> > - /* It may have the ending quote right after the file name */
> > - if (copy[p - *argptr - 2] == '"')
> > - copy[p - *argptr - 2] = 0;
> > - else
> > - copy[p - *argptr - 1] = 0;
> > - }
> > + memcpy (copy, *argptr, p - *argptr);
> > + /* It may have the ending quote right after the file name */
> > + if (is_quote_enclosed && copy[p - *argptr - 1] == '"')
> > + copy[p - *argptr - 1] = 0;
> > else
> > - {
> > - memcpy (copy, *argptr, p - *argptr);
> > - copy[p - *argptr] = 0;
> > - }
> > + copy[p - *argptr] = 0;
> >
> > /* Find that file's data. */
> > s = lookup_symtab (copy);
>
--
Daniel Jacobowitz Carnegie Mellon University
MontaVista Software Debian GNU/Linux Developer
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [RFA] Fix mi-break.exp: 'b "basics.c":16'
2002-05-09 16:05 ` Daniel Jacobowitz
@ 2002-05-09 16:32 ` Elena Zannoni
2002-05-10 12:50 ` Daniel Jacobowitz
0 siblings, 1 reply; 11+ messages in thread
From: Elena Zannoni @ 2002-05-09 16:32 UTC (permalink / raw)
To: Daniel Jacobowitz; +Cc: Michael Snyder, gdb-patches
Daniel Jacobowitz writes:
> On Thu, May 09, 2002 at 03:40:42PM -0700, Michael Snyder wrote:
> > Daniel Jacobowitz wrote:
> > >
> > > [I dislike decode_line_1. But that seems to be the general consensus...]
> > >
> > > There was a logic error in the code to handle "file":line. Several,
> > > actually. I suspect that it once worked and had bitrotten as the behavior
> > > of the function changed. For instance, at the beginning of the function:
> > >
> > > if (p[0] == '"')
> > > {
> > > is_quote_enclosed = 1;
> > > (*argptr)++;
> > > p++;
> > > }
> > >
> > > Then below it checked 'is_quote_enclosed && (**argptr) == '"').
> >
> > [line break inserted]
> >
> > > That'll only be true given a literal '"":<line>', which was not
> > > the intent of the test.
> >
> > I don't understand this statement. Between the two bits of text
> > you've named, both p and *argptr are changed. Seems to me, there
> > could be any number of characters between the two quotes.
>
> This is the part that took me the longest to work out in the first
> place, I think. `p' being changed doesn't matter here; it should end
> up point at (before?) the colon.
>
> The first if statement is at line 630. The second is at 929. Between
> the two, *argptr is only changed if we found a C++ class. It might be
> changed on "interestingly" named files, also, but if so it's purely a
> bug in the twistiness of linespec. At line 633 it points to the
> _second_ character (right after the first quote), so if **argptr ==
> '"', that means the second quote is right after the first.
As usual, I had to go and see when/why the test started failing,
and it was because a patch from March 2001, which was fixing
a core dump on the following:
(gdb) break "foo"
I have verified that your patch doesn't reintroduce the core dump.
I guess we can check that in.
Hey, any chance you can add a break "foo" in some testfile somewhere?
Probably break.exp?
Elena
>
> >
> >
> > > This patch updates the behavior, should change nothing else, and causes no
> > > regressions. OK to check in?
> > >
> > > --
> > > Daniel Jacobowitz Carnegie Mellon University
> > > MontaVista Software Debian GNU/Linux Developer
> > >
> > > 2002-04-02 Daniel Jacobowitz <drow@mvista.com>
> > >
> > > * linespec.c (decode_line_1): Check for a double quote after
> > > a filename correctly.
> > >
> > > Index: linespec.c
> > > ===================================================================
> > > RCS file: /cvs/src/src/gdb/linespec.c,v
> > > retrieving revision 1.17
> > > diff -u -p -r1.17 linespec.c
> > > --- linespec.c 2002/03/22 18:57:07 1.17
> > > +++ linespec.c 2002/04/03 00:19:30
> > > @@ -929,20 +929,12 @@ decode_line_1 (char **argptr, int funfir
> > > if ((*p == '"') && is_quote_enclosed)
> > > --p;
> > > copy = (char *) alloca (p - *argptr + 1);
> > > - if ((**argptr == '"') && is_quote_enclosed)
> > > - {
> > > - memcpy (copy, *argptr + 1, p - *argptr - 1);
> > > - /* It may have the ending quote right after the file name */
> > > - if (copy[p - *argptr - 2] == '"')
> > > - copy[p - *argptr - 2] = 0;
> > > - else
> > > - copy[p - *argptr - 1] = 0;
> > > - }
> > > + memcpy (copy, *argptr, p - *argptr);
> > > + /* It may have the ending quote right after the file name */
> > > + if (is_quote_enclosed && copy[p - *argptr - 1] == '"')
> > > + copy[p - *argptr - 1] = 0;
> > > else
> > > - {
> > > - memcpy (copy, *argptr, p - *argptr);
> > > - copy[p - *argptr] = 0;
> > > - }
> > > + copy[p - *argptr] = 0;
> > >
> > > /* Find that file's data. */
> > > s = lookup_symtab (copy);
> >
>
> --
> Daniel Jacobowitz Carnegie Mellon University
> MontaVista Software Debian GNU/Linux Developer
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [RFA] Fix mi-break.exp: 'b "basics.c":16'
2002-05-09 16:32 ` Elena Zannoni
@ 2002-05-10 12:50 ` Daniel Jacobowitz
0 siblings, 0 replies; 11+ messages in thread
From: Daniel Jacobowitz @ 2002-05-10 12:50 UTC (permalink / raw)
To: Elena Zannoni; +Cc: Michael Snyder, gdb-patches
On Thu, May 09, 2002 at 07:31:17PM -0400, Elena Zannoni wrote:
> Daniel Jacobowitz writes:
> > On Thu, May 09, 2002 at 03:40:42PM -0700, Michael Snyder wrote:
> > > Daniel Jacobowitz wrote:
> > > >
> > > > [I dislike decode_line_1. But that seems to be the general consensus...]
> > > >
> > > > There was a logic error in the code to handle "file":line. Several,
> > > > actually. I suspect that it once worked and had bitrotten as the behavior
> > > > of the function changed. For instance, at the beginning of the function:
> > > >
> > > > if (p[0] == '"')
> > > > {
> > > > is_quote_enclosed = 1;
> > > > (*argptr)++;
> > > > p++;
> > > > }
> > > >
> > > > Then below it checked 'is_quote_enclosed && (**argptr) == '"').
> > >
> > > [line break inserted]
> > >
> > > > That'll only be true given a literal '"":<line>', which was not
> > > > the intent of the test.
> > >
> > > I don't understand this statement. Between the two bits of text
> > > you've named, both p and *argptr are changed. Seems to me, there
> > > could be any number of characters between the two quotes.
> >
> > This is the part that took me the longest to work out in the first
> > place, I think. `p' being changed doesn't matter here; it should end
> > up point at (before?) the colon.
> >
> > The first if statement is at line 630. The second is at 929. Between
> > the two, *argptr is only changed if we found a C++ class. It might be
> > changed on "interestingly" named files, also, but if so it's purely a
> > bug in the twistiness of linespec. At line 633 it points to the
> > _second_ character (right after the first quote), so if **argptr ==
> > '"', that means the second quote is right after the first.
>
> As usual, I had to go and see when/why the test started failing,
> and it was because a patch from March 2001, which was fixing
> a core dump on the following:
>
> (gdb) break "foo"
>
> I have verified that your patch doesn't reintroduce the core dump.
> I guess we can check that in.
OK, committed.
> Hey, any chance you can add a break "foo" in some testfile somewhere?
> Probably break.exp?
Just posted one.
>
> Elena
>
>
> >
> > >
> > >
> > > > This patch updates the behavior, should change nothing else, and causes no
> > > > regressions. OK to check in?
> > > >
> > > > --
> > > > Daniel Jacobowitz Carnegie Mellon University
> > > > MontaVista Software Debian GNU/Linux Developer
> > > >
> > > > 2002-04-02 Daniel Jacobowitz <drow@mvista.com>
> > > >
> > > > * linespec.c (decode_line_1): Check for a double quote after
> > > > a filename correctly.
--
Daniel Jacobowitz Carnegie Mellon University
MontaVista Software Debian GNU/Linux Developer
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2002-05-10 19:50 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2002-04-03 7:58 [RFA] Fix mi-break.exp: 'b "basics.c":16' Michael Elizabeth Chastain
2002-04-03 8:20 ` Daniel Jacobowitz
-- strict thread matches above, loose matches on Subject: below --
2002-04-03 9:39 Michael Elizabeth Chastain
2002-04-03 8:39 Michael Elizabeth Chastain
2002-04-02 16:29 Daniel Jacobowitz
2002-05-09 15:31 ` Daniel Jacobowitz
2002-05-09 15:35 ` Elena Zannoni
2002-05-09 15:53 ` Michael Snyder
2002-05-09 16:05 ` Daniel Jacobowitz
2002-05-09 16:32 ` Elena Zannoni
2002-05-10 12:50 ` Daniel Jacobowitz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox