Re: [Qemu-devel] [PATCH v2 3/3] linux-user: exclude SO_TIMESTAMP cmsg_type from unsuppoted ancillary data

[Peter Maydell]

On 19 July 2012 15:33, Jing Huang <address@hidden> wrote:
> On Mon, Jul 16, 2012 at 2:45 PM, Peter Maydell <address@hidden> wrote:
>> On 16 July 2012 11:15, Jing Huang <address@hidden> wrote:
>>> This patch excludes SO_TIMESTAMP cmsg_type from unsuppoted ancillary data.
>>
>> "unsupported".
>>
>>>
>>> Signed-off-by: Jing Huang <address@hidden>
>>> ---
>>>  linux-user/syscall.c |    4 +++-
>>>  1 files changed, 3 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>>> index fc8690d..9fce57c 100644
>>> --- a/linux-user/syscall.c
>>> +++ b/linux-user/syscall.c
>>> @@ -1349,7 +1349,9 @@ static inline abi_long host_to_target_cmsg(struct 
>>> target_msghdr *target_msgh,
>>>          target_cmsg->cmsg_type = tswap32(cmsg->cmsg_type);
>>>          target_cmsg->cmsg_len = tswapal(TARGET_CMSG_LEN(len));
>>>
>>> -        if (cmsg->cmsg_level != TARGET_SOL_SOCKET || cmsg->cmsg_type != 
>>> SCM_RIGHTS) {
>>> +        if (cmsg->cmsg_level != TARGET_SOL_SOCKET ||
>>> +                    (cmsg->cmsg_type != SCM_RIGHTS &&
>>> +                                    cmsg->cmsg_type != SO_TIMESTAMP)) {
>>>              gemu_log("Unsupported ancillary data: %d/%d\n", 
>>> cmsg->cmsg_level, cmsg->cmsg_type);
>>>              memcpy(target_data, data, len);
>>>          } else {
>>
>> This is kind of ugly -- it causes us to process a SO_TIMESTAMP
>> payload (which is a 'struct timeval') in the same way as an
>> SCM_RIGHTS payload (which is an array of file descriptors).
>> This happens to work because a struct timeval is a pair of
>> 32 bit integers, and so "tswap32() all the words in the data"
>> works, but it looks pretty weird. If you want to do it this
>> way you need at least an explanatory comment and really the
>> variables 'fd', 'target_fd', 'numfds' would need to change
>> to something more generic and less fd-specific.
>>
>> -- PMM
>
>
> Hmm.. I give up this patch. It is ugly indeed. Additionally, qemu_log
> doesn't disturb the normal execution of linux-user and show error
> message either.

I think it's salvageable. As I say, you can either genericise
the "copy a pile of 32 bit values" code, or just have another
clause to the if() specifically for handling struct timeval
payload data.

-- PMM