Re: [Qemu-devel] [PATCH RFC 1/3] linux-user: add rtnetlink(7) support

[Peter Maydell]

On 14 May 2016 at 10:37, Laurent Vivier <address@hidden> wrote:
>
>
> Le 13/05/2016 à 18:40, Peter Maydell a écrit :
>> On 30 January 2016 at 22:26, Laurent Vivier <address@hidden> wrote:
>>> +    while (len > (int)sizeof(struct nlmsghdr)) {
>>
>> What is this cast to int for ?
>>
>
> I agree it seems useless, I have copied some parts from the kernel :
>
> /usr/include/linux/netlink.h
>
> #define NLMSG_OK(nlh,len) ((len) >= (int)sizeof(struct nlmsghdr) && \
> ...
>
> so the "(int)" comes from this.

Hmm. It may well be right, but I'd like to understand what
it's doing, really...

>>> +        break;
>>> +    default:
>>
>> Should we log something about an unsupported IFLA type here?
>
> Yes, I use that to debug too. But it is "fprintf()".
> Is "gemu_log()" a good choice?

It seems to be what we use elsewhere for similar logging.

>>> +    while (len >= (int)sizeof(struct rtattr)) {
>>> +        rtattr->rta_len = tswap16(rtattr->rta_len);
>>> +        rtattr->rta_type = tswap16(rtattr->rta_type);
>>> +        if (rtattr->rta_len < sizeof(struct rtattr) ||
>>> +            rtattr->rta_len > len) {
>>> +            break;
>>> +        }
>>
>> Length check after swap of len/type again.
>
> I don't understand: why "again"?

I tend to use "again" to mean "this is a similar kind of
code review comment to an issue I talked about in more
detail when it came up earlier in the patch".

thanks
-- PMM