[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <47015CDA.60109@fatooh.org>
Date: Mon, 01 Oct 2007 13:47:22 -0700
From: Corey Hickey <bugfood-ml@...ooh.org>
To: Patrick McHardy <kaber@...sh.net>,
Linux Netdev List <netdev@...r.kernel.org>
Subject: Re: [PATCH 10/10] Use nested compat attributes to pass parameters.
Patrick McHardy wrote:
> Corey Hickey wrote:
>> +
>> +#define GET_PARAM(dst, nest, compat) do { \
>> + struct rtattr *rta = tb[(nest) - 1]; \
>> + if (rta) \
>> + (dst) = RTA_GET_U32(rta); \
>> + else if ((compat)) \
>> + (dst) = (compat); \
>> +} while (0)
>
>
> An inline function and a comment why this is done would increase
> readability.
Well, I had a reason for making a macro, but it probably wasn't a good
reason. Looking now, I don't see why not to make a function. I'll see
what I can do.
>> + nest = RTA_NEST_COMPAT(skb, TCA_OPTIONS, sizeof(opt), &opt);
>> +
>> + RTA_PUT_U32(skb, TCA_SFQ_QUANTUM, q->quantum);
>> + RTA_PUT_U32(skb, TCA_SFQ_PERTURB, q->perturb_period);
>> + RTA_PUT_U32(skb, TCA_SFQ_LIMIT, q->limit);
>> + RTA_PUT_U32(skb, TCA_SFQ_DIVISOR, q->hash_divisor);
>> + RTA_PUT_U32(skb, TCA_SFQ_FLOWS, q->depth);
>> RTA_PUT(skb, TCA_OPTIONS, sizeof(opt), &opt);
>
>
> This is wrong, RTA_NEST_COMPAT already dumps the structure.
You mean that last line (RTA_PUT) is superfluous, right? I can't see a
reason for it to be there, so I must have just forgotten to delete it
from the original code.
If I'm wrong, I might need a little hand-holding here. My understanding
of all the RTA stuff is a bit shaky.
Much thanks for the review. I'll make a new set of patches soon.
-Corey
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists