On Mon, Jan 24, 2011 at 02:12:28AM +0100, Javier Martinez Canillas wrote:
> + case IOCTL_BCM_CNTRLMSG_MASK:
> + {
This should be indented two tabs. One for the function and one for the
switch statement.
> + unsigned long RxCntrlMsgBitMask = 0;
> +
> + /* Copy Ioctl Buffer structure */
> + Status = copy_from_user(&IoBuffer, argp, sizeof(IOCTL_BUFFER));
> + if (Status) {
> + BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, OSAL_DBG, DBG_LVL_ALL,
> + "copy of Ioctl buffer is failed from user space");
> + Status = -EFAULT;
> + break;
> + }
>
> - /* Copy Ioctl Buffer structure */
> - Status = copy_from_user(&IoBuffer, argp, sizeof(IOCTL_BUFFER));
> - if(Status)
> - {
> - BCM_DEBUG_PRINT(Adapter,DBG_TYPE_OTHERS, OSAL_DBG, DBG_LVL_ALL,"copy of Ioctl buffer is failed from user space");
> - break;
> - }
> + if (IoBuffer.InputLength > sizeof(unsigned long)) {
Personally I would say:
if (IoBuffer.InputLength != sizeof(unsigned long)) {
There is no other size that makes sense here. It's more helpful to
return an error directly.
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
