This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] Fix mi-break.exp: 'b "basics.c":16'


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]