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 [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to