lkml.org 
[lkml]   [2000]   [Nov]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: OOPS loading cs46xx module, test11-pre1
Christoph Hellwig wrote:
>
> On Thu, Nov 09, 2000 at 12:02:45PM -0500, Jeff Garzik wrote:
> > > I have an (untested) update for the cs46xx driver in Linux 2.4.
> > > It includes Nils' 2.2 changes, use of initcalls (so compiled-in
> > > should work) and use of the 2.4 PCI interface.
> >
> > Patch Generally looks ok. Comments:
> >
> > 1) This code is weird:
> > > if (copy_to_user(buffer, dmabuf->rawbuf + swptr, cnt)) {
> > >- if (!ret) ret = -EFAULT;
> > >- return ret;
> > >+ if (!ret)
> > >+ ret = -EFAULT;
> > >+ break;
> > > }
> >
> > If you have an error condition (ret != 0), then you should not attempt
> > the copy_to_user at all.
> > If you do not have an error condition, you should unconditionally assign
> > -EFAULT to 'ret'.
>
> Makes sense. But I did not change the logic at all ...
> Ask the Author (Alan or Nils) what this is about.

Now that I have looked at the code around it, it looks ok. It returns
the bytes copied to userspace so far, like a good read function should.

> > 6) remove check for pci_present(), redundant
>
> Lot's of pci drivers do this. Anyway I will removed this in the next version.

It's been redundant since the 2.1.x days, maybe even before that. There
-might- be a rare case where a driver really needs to check for the
presence of a PCI BIOS. Maybe. But for all cases where you have

if (!pci_present()) probe_pci=0;
/* ... */
if (probe_pci) {
pci_register_driver(...)
or
pdev = pci_find_device(...);
}

you can remove the pci_present check. All PCI probe functions return
sane values in the absence of PCI support (or PCI devices).

> > of course for cs46xx, you will want to keep the version printk...
> >
> > 8) xxx_MODULE_NAME was a dumb and overly-lengthy creation of mine. Use
> > instead MODNAME.
>
> It doesn't really matter ...

besides perpetuating a bad idea, that is.

Jeff


--
Jeff Garzik |
Building 1024 | Would you like a Twinkie?
MandrakeSoft |
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 12:45    [W:0.049 / U:5.044 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site