Re: Patch?: linux-2.5.59/sound/soundcore.c referenced non-existant errno variable

From: Petr Vandrovec (vandrove@vc.cvut.cz)
Date: Fri Jan 17 2003 - 22:13:19 EST


On Fri, Jan 17, 2003 at 08:49:36PM -0600, Raja R Harinath wrote:
> Hi,
>
> "Adam J. Richter" <adam@yggdrasil.com> writes:
>
> > linux-2.5.59/sound/sound_firmware.c attempts to use the
> > user level system call interface from the kernel, which I understand
> > works on i386 and perhaps all architectures, but requires a variable
> > named "errno."
>
> Which is provided in-kernel (not for modules) by 'lib/errno.c'.

Not safe. We should either remove errno from kernel syscall wrappers
completely when building __KERNEL__ (just return -1 and nothing more
specific), or even disallow use of unistd.h wrappers from kernel
completely (which is best solution IMHO). Or we must make errno
per-thread variable, which is unnecessary complication...

> > (Actually, it mixed things like close() and sys_close(), but that's
> > beside the point.)
>
> Those are provided by <linux/unistd.h>, with __KERNEL_SYSCALLS__
> defined.
>
> > I could just declare a "static int errno;" in the file,
>
> That was originally there, but removed in 2.5.57 IIRC.
> <linux/unistd.h> has 'extern int errno;' -- so 'static int errno;'
> would be a bug.

I sent patch below few days ago, but I received only confirmations that
fixes compilation - nobody with hardware which uses this firmware
loader confirmed that it works.

Today morning I decided to send it to Linus anyway, as at worst
less broken is better than more broken, but unfortunately Linus was
already at airport :-(

BTW, static int errno is by far best solution if you do not agree with
patch below: due to toolchain behavior soundcore will use its own
errno for syscall wrappers it uses, and it is nearest to the behavior
we wanted...
                                                Best regards,
                                                        Petr Vandrovec
                                                        vandrove@vc.cvut.cz

diff -ur linux-2.5.58-c952.dist/sound/sound_firmware.c linux-2.5.58-c952/sound/sound_firmware.c
--- linux-2.5.58-c952.dist/sound/sound_firmware.c 2003-01-16 22:33:32.000000000 +0100
+++ linux-2.5.58-c952/sound/sound_firmware.c 2003-01-17 14:54:40.000000000 +0100
@@ -1,47 +1,46 @@
 #include <linux/vmalloc.h>
-#define __KERNEL_SYSCALLS__
 #include <linux/module.h>
 #include <linux/fs.h>
 #include <linux/mm.h>
 #include <linux/slab.h>
-#include <linux/unistd.h>
 #include <asm/uaccess.h>
 
 static int do_mod_firmware_load(const char *fn, char **fp)
 {
- int fd;
+ struct file* filp;
         long l;
         char *dp;
+ loff_t pos;
 
- fd = open(fn, 0, 0);
- if (fd == -1)
+ filp = filp_open(fn, 0, 0);
+ if (IS_ERR(filp))
         {
                 printk(KERN_INFO "Unable to load '%s'.\n", fn);
                 return 0;
         }
- l = lseek(fd, 0L, 2);
+ l = filp->f_dentry->d_inode->i_size;
         if (l <= 0 || l > 131072)
         {
                 printk(KERN_INFO "Invalid firmware '%s'\n", fn);
- sys_close(fd);
+ filp_close(filp, NULL);
                 return 0;
         }
- lseek(fd, 0L, 0);
         dp = vmalloc(l);
         if (dp == NULL)
         {
                 printk(KERN_INFO "Out of memory loading '%s'.\n", fn);
- sys_close(fd);
+ filp_close(filp, NULL);
                 return 0;
         }
- if (read(fd, dp, l) != l)
+ pos = 0;
+ if (vfs_read(filp, dp, l, &pos) != l)
         {
                 printk(KERN_INFO "Failed to read '%s'.\n", fn);
                 vfree(dp);
- sys_close(fd);
+ filp_close(filp, NULL);
                 return 0;
         }
- close(fd);
+ filp_close(filp, NULL);
         *fp = dp;
         return (int) l;
 }
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Thu Jan 23 2003 - 22:00:17 EST