Re: [Qemu-devel] [PATCH] linux-user: fix convertion of flock/flock64 l_type field

[Laurent Vivier]

Le 08/05/2018 à 20:55, Eric Blake a écrit :
> On 05/08/2018 12:08 PM, Max Filippov wrote:
>> On Tue, May 8, 2018 at 9:36 AM, Laurent Vivier <address@hidden> wrote:
> 
> In the subject, s/convertion/conversion/
> 
>>> As l_type values (F_RDLCK, F_WRLCK, F_UNLCK, F_EXLCK, F_SHLCK)
>>> are not bitmasks, we can't use target_to_host_bitmask() and
>>> host_to_target_bitmask() to convert them.
>>>
>>> Introduce target_to_host_flock() and host_to_target_flock()
>>> to convert values between host and target.
>>>
> 
>>> +static unsigned int host_to_target_flock(unsigned int type)
>>> +{
>>> +    switch (type) {
>>> +#define TRANSTBL_CONVERT(a) case a: return TARGET_##a;
>>> +    TRANSTBL_CONVERT(F_RDLCK)
>>> +    TRANSTBL_CONVERT(F_WRLCK)
>>> +    TRANSTBL_CONVERT(F_UNLCK)
>>> +    TRANSTBL_CONVERT(F_EXLCK)
>>> +    TRANSTBL_CONVERT(F_SHLCK)
>>> +#undef  TRANSTBL_CONVERT
>>> +    }
>>> +    return type;
>>> +}
>>>
>>
>> There's a duplication. Wouldn't it be better if it was done like the
>> following:
>>
>> #define FLOCK_TRANSTBL \
>>      switch (type) {
>>      TRANSTBL_CONVERT(F_RDLCK) \
> 
> If you do this, I'd lean towards omitting the trailing ; from
> TRANSTBL_CONVERT() and sticking it in FLOCK_TRANSTBL instead (it looks
> weird to see a statement-like macro called without a ';').
> 

I have included this patch in the series "linux-user: fix sparc32plus"
and missed your comments. I'll update this patch in the v2 of the series.

Thanks,
Laurent