Messages in this thread | | | Subject | Re: [PATCH 1/2 v4] Driver for Freescale 8610 and 5121 DIU | From | York Sun <> | Date | Fri, 28 Mar 2008 13:18:49 -0500 |
| |
On Fri, 2008-03-28 at 20:48 +0300, Anton Vorontsov wrote: > > >+ if (copy_from_user(&pix_fmt, (void __user *)arg, > > >+ sizeof(pix_fmt))) > > > > OK, you fixed the cast here... > > Maybe better would be to define "buf" at the top of this function as > void __user *buf = (void __user *)arg;, i.e. just once?
I think it is a good idea.
> > > > > >+ return -EFAULT; > > >+ ad->pix_fmt = pix_fmt; > > >+ pr_debug("Set pixel format to 0x%08x\n", ad->pix_fmt); > > >+ break; > > >+ case MFB_GET_PIXFMT: > > >+ if (!arg) > > >+ return -EINVAL; > > This if (!arg) appears to be everywhere except default: label, maybe > this could be placed on top too? Will do that, too.
York
| |