Re: [RFC v3 2/2] v4l: Implement compat functions for enum to __u32 change

[Mauro Carvalho Chehab <mchehab@redhat.com>]

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