lkml.org 
[lkml]   [2008]   [Mar]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/2 v4] Driver for Freescale 8610 and 5121 DIU
On Fri, Mar 28, 2008 at 12:30:29PM -0500, Scott Wood wrote:
> York Sun wrote:
> >+static int fsl_diu_ioctl(struct fb_info *info, unsigned int cmd,
> >+ unsigned long arg)
> >+{
> >+ struct mfb_info *mfbi = info->par;
> >+ struct diu_ad *ad = mfbi->ad;
> >+ struct mfb_chroma_key ck;
> >+ unsigned char global_alpha;
> >+ struct aoi_display_offset aoi_d;
> >+ __u32 pix_fmt;
> >+
> >+ switch (cmd) {
> >+ case MFB_SET_PIXFMT:
> >+ if (!arg)
> >+ return -EINVAL;
> >+ 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?

>
> >+ 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?

> >+ pix_fmt = ad->pix_fmt;
> >+ if (copy_to_user((void *)arg, &pix_fmt, sizeof(pix_fmt)))
> >+ return -EFAULT;
> >+ pr_debug("get pixel format 0x%08x\n", ad->pix_fmt);
> >+ break;
> >+ case MFB_SET_AOID:
> >+ if (!arg)
> >+ return -EINVAL;
> >+ if (copy_from_user(&aoi_d, (void *)arg, sizeof(aoi_d)))
> >+ return -EFAULT;
>
> ...but not anywhere else. All user pointers should have a __user
> annotation.

--
Anton Vorontsov
email: cboumailru@gmail.com
irc://irc.freenode.net/bd2


\
 
 \ /
  Last update: 2008-03-28 18:51    [W:0.064 / U:0.916 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site