[PATCH] fix of the get_id fix

[PATCH] fix of the get_id fix

[Clemens Ladisch]

Jaroslav Kysela wrote:
> revision 1.6
> date: 2002/11/28 13:54:34;  author: perex;  state: Exp;  lines: +3 -4
>
> Fixed parsiong of id where dst is NULL
>
> Index: initval.h
> ===================================================================
> RCS file: /cvsroot/alsa/alsa-kernel/include/initval.h,v
> retrieving revision 1.5
> retrieving revision 1.6
> diff -u -r1.5 -r1.6
> --- initval.h   23 Nov 2002 10:41:53 -0000      1.5
> +++ initval.h   28 Nov 2002 13:54:34 -0000      1.6
> @@ -156,10 +156,9 @@
>         for (s = *str; isalpha(*s) || isdigit(*s) || *s == '_'; s++);
>         if (s != *str) {
>                 *dst = (char *)kmalloc(s - *str, GFP_KERNEL);
> -               if ((d = *dst) != NULL) {
> -                       s = *str;
> -                       while (isalpha(*s) || isdigit(*s) || *s == '_')
> +               s = *str; d = *dst;
> +               while (isalpha(*s) || isdigit(*s) || *s == '_')
> +                       if (d != NULL)
>                                 *d++ = *s++;
> -               }
>         }
>         *str = s;

I'm not sure I understand what this patch tries to fix. Revision 1.5 seems
to work when kmalloc fails (it skips the input field and returns NULL),
but revision 1.6 goes into an infinite loop because s isn't incremented
when d == NULL.

And IMNSHO the returned string should be zero-terminated.


-- 
Clemens



Index: alsa-kernel/include/initval.h
===================================================================
RCS file: /cvsroot/alsa/alsa-kernel/include/initval.h,v
retrieving revision 1.6
diff -u -r1.6 initval.h
--- alsa-kernel/include/initval.h	28 Nov 2002 13:54:34 -0000	1.6
+++ alsa-kernel/include/initval.h	2 Dec 2002 08:48:55 -0000
@@ -155,11 +155,13 @@
 		return 0;
 	for (s = *str; isalpha(*s) || isdigit(*s) || *s == '_'; s++);
 	if (s != *str) {
-		*dst = (char *)kmalloc(s - *str, GFP_KERNEL);
+		*dst = (char *)kmalloc(s - *str + 1, GFP_KERNEL);
-		s = *str; d = *dst;
-		while (isalpha(*s) || isdigit(*s) || *s == '_')
-			if (d != NULL)
+		if ((d = *dst) != NULL) {
+			s = *str;
+			while (isalpha(*s) || isdigit(*s) || *s == '_')
 				*d++ = *s++;
+			*d = '\0';
+		}
 	}
 	*str = s;
 	if (*s == ',') {



-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf

Re: [PATCH] fix of the get_id fix

[Takashi Iwai]

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

At Mon, 02 Dec 2002 11:39:34 +0100 (MET),
Clemens Ladisch wrote:
> 
> Jaroslav Kysela wrote:
> > revision 1.6
> > date: 2002/11/28 13:54:34;  author: perex;  state: Exp;  lines: +3 -4
> >
> > Fixed parsiong of id where dst is NULL
> >
> > Index: initval.h
> > ===================================================================
> > RCS file: /cvsroot/alsa/alsa-kernel/include/initval.h,v
> > retrieving revision 1.5
> > retrieving revision 1.6
> > diff -u -r1.5 -r1.6
> > --- initval.h   23 Nov 2002 10:41:53 -0000      1.5
> > +++ initval.h   28 Nov 2002 13:54:34 -0000      1.6
> > @@ -156,10 +156,9 @@
> >         for (s = *str; isalpha(*s) || isdigit(*s) || *s == '_'; s++);
> >         if (s != *str) {
> >                 *dst = (char *)kmalloc(s - *str, GFP_KERNEL);
> > -               if ((d = *dst) != NULL) {
> > -                       s = *str;
> > -                       while (isalpha(*s) || isdigit(*s) || *s == '_')
> > +               s = *str; d = *dst;
> > +               while (isalpha(*s) || isdigit(*s) || *s == '_')
> > +                       if (d != NULL)
> >                                 *d++ = *s++;
> > -               }
> >         }
> >         *str = s;
> 
> I'm not sure I understand what this patch tries to fix. Revision 1.5 seems
> to work when kmalloc fails (it skips the input field and returns NULL),
> but revision 1.6 goes into an infinite loop because s isn't incremented
> when d == NULL.

i guess s should be proceeded even if kmalloc fails, i.e. skip the
word.

> 
> And IMNSHO the returned string should be zero-terminated.

yep.


Takashi

[-- Attachment #2: initval-fix.dif --]
[-- Type: application/octet-stream, Size: 701 bytes --]

Index: alsa-kernel/include/initval.h
===================================================================
--- alsa-kernel/include/initval.h	29 Nov 2002 14:35:01 -0000	1.5
+++ alsa-kernel/include/initval.h	2 Dec 2002 11:30:58 -0000
@@ -155,11 +155,13 @@
 		return 0;
 	for (s = *str; isalpha(*s) || isdigit(*s) || *s == '_'; s++);
 	if (s != *str) {
-		*dst = (char *)kmalloc(s - *str, GFP_KERNEL);
+		*dst = (char *)kmalloc(s - *str + 1, GFP_KERNEL);
 		s = *str; d = *dst;
-		while (isalpha(*s) || isdigit(*s) || *s == '_')
+		for (; isalpha(*s) || isdigit(*s) || *s == '_'; s++)
 			if (d != NULL)
-				*d++ = *s++;
+				*d++ = *s;
+		if (d != NULL)
+			*d = '\0';
 	}
 	*str = s;
 	if (*s == ',') {