card_id() in initval.h

card_id() in initval.h

[Andre Batista de Oliveira]

I'm facing some problems passing the ALSA drivers'
parameters from the kernel command line.

When I try to specify the ID string for the card
(snd-sgalaxy=1,0,Waverider,0x220,0x530,10,3),
the kernel just reboots immediately, even before printing anything!

But when I leave the ID string unspecified
(snd-sgalaxy=1,0,,0x220,0x530,10,3), it boots and works fine,
and the card gets the default "Galaxy" ID.

I believe this has something to do with the kmalloc() call
in the get_id() function in the initval.h include file.
Perhaps kmalloc() can not be used at this early boot stage?!?
I fixed it by replacing kmalloc() with alloc_bootmem(),
and now it works, but there's probably a better way to do it!

Does this make any sense? Is this a known bug? Is this a bug, at all?
Thanks.

Here's the diff:

--- initval.h.orig      Tue Dec 23 16:56:26 2003
+++ initval.h   Tue Dec 23 18:00:10 2003
@@ -136,25 +136,23 @@
 #if defined(SNDRV_GET_ID) && !defined(MODULE)
 #include <linux/ctype.h>
 #include <linux/init.h>
+#include <linux/bootmem.h>
 static int __init get_id(char **str, char **dst)
 {
-       char *s, *d;
+       char *s;
 
        if (!(*str) || !(**str))
                return 0;
        for (s = *str; isalpha(*s) || isdigit(*s) || *s == '_'; s++);
        if (s != *str) {
-               *dst = (char *)kmalloc((s - *str) + 1, GFP_KERNEL);
-               s = *str; d = *dst;
-               while (isalpha(*s) || isdigit(*s) || *s == '_') {
-                       if (d != NULL)
-                               *d++ = *s;
-                       s++;
-               }
-               if (d != NULL)
+               char *d = alloc_bootmem(s - *str + 1);
+               if (d != NULL) {
+                       *dst = d;
+                       while (s != *str) *d++ = *(*str)++;
                        *d = '\0';
+               } else
+                       *str = s;
        }
-       *str = s;
        if (*s == ',') {
                (*str)++;
                return 2;



-- 
André Batista de Oliveira
http://mega.ist.utl.pt/~abo


-------------------------------------------------------
This SF.net email is sponsored by: IBM Linux Tutorials.
Become an expert in LINUX or just sharpen your skills.  Sign up for IBM's
Free Linux Tutorials.  Learn everything from the bash shell to sys admin.
Click now! http://ads.osdn.com/?ad_id\x1278&alloc_id371&op=click

Re: card_id() in initval.h

[Jaroslav Kysela]

On Tue, 23 Dec 2003, Andre Batista de Oliveira wrote:

> I'm facing some problems passing the ALSA drivers'
> parameters from the kernel command line.
> 
> When I try to specify the ID string for the card
> (snd-sgalaxy=1,0,Waverider,0x220,0x530,10,3),
> the kernel just reboots immediately, even before printing anything!
> 
> But when I leave the ID string unspecified
> (snd-sgalaxy=1,0,,0x220,0x530,10,3), it boots and works fine,
> and the card gets the default "Galaxy" ID.

Thanks for the bug report. Could you try this "official" patch?

Index: initval.h
===================================================================
RCS file: /cvsroot/alsa/alsa-kernel/include/initval.h,v
retrieving revision 1.17
diff -u -r1.17 initval.h
--- initval.h	30 Oct 2003 14:21:07 -0000	1.17
+++ initval.h	27 Dec 2003 09:00:51 -0000
@@ -138,27 +138,24 @@
 #include <linux/init.h>
 static int __init get_id(char **str, char **dst)
 {
-	char *s, *d;
+	char *s;
 
 	if (!(*str) || !(**str))
 		return 0;
 	for (s = *str; isalpha(*s) || isdigit(*s) || *s == '_'; s++);
 	if (s != *str) {
-		*dst = (char *)kmalloc((s - *str) + 1, GFP_KERNEL);
-		s = *str; d = *dst;
-		while (isalpha(*s) || isdigit(*s) || *s == '_') {
-			if (d != NULL)
-				*d++ = *s;
-			s++;
+		int len = s - *str;
+		char *d = (char *)alloc_bootmem(len + 1);
+		if (d != NULL) {
+			memcpy(*dst = d, *str, len);
+			d[len] = '\0';
 		}
-		if (d != NULL)
-			*d = '\0';
 	}
-	*str = s;
 	if (*s == ',') {
-		(*str)++;
+		*str = s + 1;
 		return 2;
 	}
+	*str = s;
 	return 1;
 }
 #endif

					Jaroslav

-----
Jaroslav Kysela <perex@suse.cz>
Linux Kernel Sound Maintainer
ALSA Project, SuSE Labs


-------------------------------------------------------
This SF.net email is sponsored by: IBM Linux Tutorials.
Become an expert in LINUX or just sharpen your skills.  Sign up for IBM's
Free Linux Tutorials.  Learn everything from the bash shell to sys admin.
Click now! http://ads.osdn.com/?ad_id=1278&alloc_id=3371&op=click

Re: card_id() in initval.h

[Andre Batista de Oliveira]

Jaroslav Kysela <perex@suse.cz> writes:

> On Tue, 23 Dec 2003, Andre Batista de Oliveira wrote:
> > When I try to specify the ID string for the card
> > (snd-sgalaxy=1,0,Waverider,0x220,0x530,10,3),
> > the kernel just reboots immediately, even before printing anything!
> 
> Thanks for the bug report. Could you try this "official" patch?
> 
> RCS file: /cvsroot/alsa/alsa-kernel/include/initval.h,v
> retrieving revision 1.17
> diff -u -r1.17 initval.h
> --- initval.h	30 Oct 2003 14:21:07 -0000	1.17
> +++ initval.h	27 Dec 2003 09:00:51 -0000

Ok, it works!
I tried it in a 2.6.0 kernel, which has the exact same get_id()
function as the ALSA CVS 1.17 revision of initval.h.

You should also add #include <linux/bootmem.h> to avoid compilation
warning: implicit declaration of function `alloc_bootmem'.

Now I was thinking if there shouldn't be a free_bootmem()
somewhere after the card id string parameter has been copied to the
id field of the struct _snd_card ?!?
Maybe right after strlcpy(card->id, xid, sizeof(card->id)), in the
snd_card_new() function of core/init.c ?!?
Or perhaps it should be the drivers' responsibility to do a
free_bootmem() after calling snd_card_new() ?!?
Then again, this is not a big memory leak to worry about...

-- 
André Batista de Oliveira


-------------------------------------------------------
This SF.net email is sponsored by: IBM Linux Tutorials.
Become an expert in LINUX or just sharpen your skills.  Sign up for IBM's
Free Linux Tutorials.  Learn everything from the bash shell to sys admin.
Click now! http://ads.osdn.com/?ad_id\x1278&alloc_id371&op=click