bug#5601: 23.1; etags Scheme_functions past \0 terminator

bug#5601: 23.1; etags Scheme_functions past \0 terminator

[Kevin Ryde]

[-- Attachment #1: Type: text/plain, Size: 628 bytes --]

In etags.c Scheme_functions, I think the loop

    while (notinname (*bp))
      bp++;

will take bp past the '\0' string terminator, because '\0' is a
notinname().

I can't spot any obvious ill effect, only that a line of only

    (define

is tagged, perhaps depending on what was on the line before it.  In any
case doesn't sound good to look into possibly uninitialized parts of the
input buffer.  (Another helper skip_notinname() to try to be clearer
than a double-negative loop :-)

2010-02-19  Kevin Ryde  <user42@zip.com.au>

	* etags.c (Scheme_functions): Don't go past '\0' terminator.
	(skip_notinname): New helper.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: etags.c.scheme-terminator.diff --]
[-- Type: text/x-diff, Size: 686 bytes --]

--- etags.c.~3.93.~	2009-11-29 08:42:32.000000000 +1100
+++ etags.c	2010-02-19 11:04:42.000000000 +1100
@@ -4989,6 +4989,16 @@
  *          (set! xyzzy
  * Original code by Ken Haase (1985?)
  */
+
+static char *
+skip_notinname (char *cp)
+{
+  /* '\0' is a notinname(), don't continue past it */
+  while (*cp && notinname (*cp))
+    cp++;
+  return cp;
+}
+
 static void
 Scheme_functions (inf)
      FILE *inf;
@@ -5001,8 +5011,7 @@
 	{
 	  bp = skip_non_spaces (bp+4);
 	  /* Skip over open parens and white space */
-	  while (notinname (*bp))
-	    bp++;
+	  bp = skip_notinname (bp);
 	  get_tag (bp, NULL);
 	}
       if (LOOKING_AT (bp, "(SET!") || LOOKING_AT (bp, "(set!"))

[-- Attachment #3: Type: text/plain, Size: 733 bytes --]




In GNU Emacs 23.1.1 (i486-pc-linux-gnu, GTK+ Version 2.16.5)
 of 2009-09-14 on raven, modified by Debian
configured using `configure  '--build=i486-linux-gnu' '--host=i486-linux-gnu' '--prefix=/usr' '--sharedstatedir=/var/lib' '--libexecdir=/usr/lib' '--localstatedir=/var/lib' '--infodir=/usr/share/info' '--mandir=/usr/share/man' '--with-pop=yes' '--enable-locallisppath=/etc/emacs23:/etc/emacs:/usr/local/share/emacs/23.1/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/23.1/site-lisp:/usr/share/emacs/site-lisp:/usr/share/emacs/23.1/leim' '--with-x=yes' '--with-x-toolkit=gtk' '--with-toolkit-scroll-bars' 'build_alias=i486-linux-gnu' 'host_alias=i486-linux-gnu' 'CFLAGS=-DDEBIAN -g -O2' 'LDFLAGS=-g' 'CPPFLAGS=''

bug#5601: 23.1; etags Scheme_functions past \0 terminator

[Francesco Potortì]

>In etags.c Scheme_functions, I think the loop
>
>    while (notinname (*bp))
>      bp++;
>
>will take bp past the '\0' string terminator, because '\0' is a
>notinname().

Yes, it appears so.  This is a long-standing bug.  Thanks for spotting
it.

>(Another helper skip_notinname() to try to be clearer
>than a double-negative loop :-)

I don't think a helper function is granted here.  There is a single use
of this construct in the file, and a lot of different constructs: we
have not a helper function for each of them.  Let's try to stick to the
comomn ones only, else we will only add to the confusion.  So, please do
not define a helper function for this case.

Again, I can do the change myself, or else you can do it yourself, as
you like.

>2010-02-19  Kevin Ryde  <user42@zip.com.au>
>
>	* etags.c (Scheme_functions): Don't go past '\0' terminator.
>	(skip_notinname): New helper.
>
>--- etags.c.~3.93.~	2009-11-29 08:42:32.000000000 +1100
>+++ etags.c	2010-02-19 11:04:42.000000000 +1100
>@@ -4989,6 +4989,16 @@
>  *          (set! xyzzy
>  * Original code by Ken Haase (1985?)
>  */
>+
>+static char *
>+skip_notinname (char *cp)
>+{
>+  /* '\0' is a notinname(), don't continue past it */
>+  while (*cp && notinname (*cp))
>+    cp++;
>+  return cp;
>+}
>+
> static void
> Scheme_functions (inf)
>      FILE *inf;
>@@ -5001,8 +5011,7 @@
> 	{
> 	  bp = skip_non_spaces (bp+4);
> 	  /* Skip over open parens and white space */
>-	  while (notinname (*bp))
>-	    bp++;
>+	  bp = skip_notinname (bp);
> 	  get_tag (bp, NULL);
> 	}
>       if (LOOKING_AT (bp, "(SET!") || LOOKING_AT (bp, "(set!"))
>
>
>
>In GNU Emacs 23.1.1 (i486-pc-linux-gnu, GTK+ Version 2.16.5)
> of 2009-09-14 on raven, modified by Debian
>configured using `configure  '--build=i486-linux-gnu' '--host=i486-linux-gnu' '--prefix=/usr' '--sharedstatedir=/var/lib' '--libexecdir=/usr/lib' '--localstatedir=/var/lib' '--infodir=/usr/share/info' '--mandir=/usr/share/man' '--with-pop=yes' '--enable-locallisppath=/etc/emacs23:/etc/emacs:/usr/local/share/emacs/23.1/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/23.1/site-lisp:/usr/share/emacs/site-lisp:/usr/share/emacs/23.1/leim' '--with-x=yes' '--with-x-toolkit=gtk' '--with-toolkit-scroll-bars' 'build_alias=i486-linux-gnu' 'host_alias=i486-linux-gnu' 'CFLAGS=-DDEBIAN -g -O2' 'LDFLAGS=-g' 'CPPFLAGS=''

bug#5601: 23.1; etags Scheme_functions past \0 terminator

[Chong Yidong]

> >In etags.c Scheme_functions, I think the loop
> >
> >    while (notinname (*bp))
> >      bp++;
> >
> >will take bp past the '\0' string terminator, because '\0' is a
> >notinname().
>
> Yes, it appears so.  This is a long-standing bug.  Thanks for spotting
> it.
>
> >(Another helper skip_notinname() to try to be clearer
> >than a double-negative loop :-)
>
> I don't think a helper function is granted here.  There is a single use
> of this construct in the file, and a lot of different constructs: we
> have not a helper function for each of them.

I've checked in the patch, without the helper function.  Thanks.