lkml.org 
[lkml]   [2007]   [Dec]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [patch 07/18] v4l: nopage
On Sat, 8 Dec 2007 10:15:08 +0100 Ingo Molnar <mingo@elte.hu> wrote:

>
> * Andrew Morton <akpm@linux-foundation.org> wrote:
>
> > On Wed, 05 Dec 2007 18:15:54 +1100
> > npiggin@suse.de wrote:
> >
> > > +static int
> > > +videobuf_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
> > > {
> > > struct page *page;
> > >
> > > - dprintk(3,"nopage: fault @ %08lx [vma %08lx-%08lx]\n",
> > > - vaddr,vma->vm_start,vma->vm_end);
> > > - if (vaddr > vma->vm_end)
> > > - return NOPAGE_SIGBUS;
> > > + dprintk(3,"fault: fault @ %08lx [vma %08lx-%08lx]\n",
> > > + (unsigned long)vmf->virtual_address,vma->vm_start,vma->vm_end);
> > > page = alloc_page(GFP_USER | __GFP_DMA32);
> > > if (!page)
> > > - return NOPAGE_OOM;
> > > + return VM_FAULT_OOM;
> > > clear_user_page(page_address(page), vaddr, page);
> >
> > This didn't compile on sparc64 because `vaddr' is undefined.
> >
> >
> > Let us see why:
> >
> > #define clear_user_page(page, vaddr, pg) clear_page(page)
> > #define copy_user_page(to, from, vaddr, pg) copy_page(to, from)
> >
> > #define __alloc_zeroed_user_highpage(movableflags, vma, vaddr) \
> > alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO | movableflags, vma, vaddr)
> >
> > root cause: lack of argument checking on x86 due to stupid macros.
> >
> >
> > Could someone *please* start a little project of extirpating this
> > utter brain damage? Convert those macros to typechecked static
> > inlines on x86 (at least) so this sort of thing (which happens again
> > and again and again) is lessened?

We should fix existing stuff, like this.

> i wanted to write a reply to suggest a checkpatch policy for this. When
> i noticed this sentence at the end of your mail:
>
> > macros are such miserable things. I wonder if we could get checkpatch
> > to help out here?
>
> /me too :-)
>
> any policy that gets into checkpatch.pl's default output is a policy for
> arch/x86 patch merging. (as long as it's not a false positive) And
> because we do all these unifications the effects of checkpatch.pl
> permeate basically every aspect of arch/x86.
>
> one approach would be to make new macros in include/* a no-no. That
> would hurt a few of the legitimate uses though, so maybe a useful
> refinement would be to check the structure of macros: are arguments used
> twice (side-effect), are arguments unused (typechecking dager), are
> arguments cast (type-loss danger), etc. Looks very hard to implement
> though :-/ Andy, what do you think?

I think whining about anything which matches

#define foo(...) bar(

would be a decent start.

grep '^[ ]*#[ ]*define[ ][ ]*[^(]*[(][^)]*[)][ ]*[a-zA-Z]' include/asm-x86/*.h

(hey, that worked on the first attempt)

Lots of falsies tho.


\
 
 \ /
  Last update: 2007-12-08 11:19    [W:0.478 / U:0.028 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site