Em 02-05-2012 20:38, Laurent Pinchart escreveu:
> Hi Mauro,
>
> On Wednesday 02 May 2012 19:32:23 Mauro Carvalho Chehab wrote:
>> Em 02-05-2012 16:13, Sakari Ailus escreveu:
>>> Implement compat functions to provide conversion between structs
>>> containing enums and those not. The functions are intended to be removed
>>> when the support for such old binaries is no longer necessary.
>>
>> This is not a full review of this patch, as checking the full logic
>> will consume some time.
>>
>> This is just a few points to consider.
>
> [snip]
>
>>> -video_usercopy(struct file *file, unsigned int cmd, unsigned long arg,
>>> +video_usercopy(struct file *file, unsigned int compat_cmd, unsigned long
>>> arg,>
>>> v4l2_kioctl func)
>>>
>>> {
>>>
>>> char sbuf[128];
>>>
>>> @@ -2401,6 +3048,7 @@ video_usercopy(struct file *file, unsigned int cmd,
>>> unsigned long arg,>
>>> size_t array_size = 0;
>>> void __user *user_ptr = NULL;
>>> void **kernel_ptr = NULL;
>>>
>>> + unsigned int cmd = get_non_compat_cmd(compat_cmd);
>>
>> This will put a penalty on archs where sizeof(u32) == sizeof(enum), with
>> covers most of the cases.
>>
>> My suggestion is to, instead, call it at the end of __video_do_ioctl, at
>> the "default" clause, with a recursive call to __video_do_ioctl().
>
> Would that work for "has_array_args" ioctls ? video_usercopy() won't copy the
> array. The compat code could possibly handle that though.
>
> What about making get_non_compat_cmd() return its argument directly when
> sizeof(__u32) == sizeof(enum) ? The performance impact should be negligible.
Good idea. GCC optimizer will probably just discard the entire code, by merging
'compat_cmd' var with 'cmd' var.
Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
