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

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

[Daniel Jacobowitz]

[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);

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

[Daniel Jacobowitz]

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

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

[Elena Zannoni]

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

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

[Michael Snyder]

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);

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

[Daniel Jacobowitz]

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

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

[Elena Zannoni]

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

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

[Daniel Jacobowitz]

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

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

[Michael Elizabeth Chastain]

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.

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

[Michael Elizabeth Chastain]

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

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

[Daniel Jacobowitz]

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

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

[Michael Elizabeth Chastain]

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.