On 14 May 2016 at 10:37, Laurent Vivier <[email protected]> wrote:
>
>
> Le 13/05/2016 à 18:40, Peter Maydell a écrit :
>> On 30 January 2016 at 22:26, Laurent Vivier <[email protected]> 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
