2009/6/4 Patrick McHardy <kaber@xxxxxxxxx>: > Dmitry Eremin-Solenikov wrote: >> >> 2009/6/4 Johannes Berg <johannes@xxxxxxxxxxxxxxxx>: >>> >>> On Wed, 2009-06-03 at 16:27 +0400, Dmitry Eremin-Solenikov wrote: >>> >>>>>> +static int ieee802154_nl_put_failure(struct sk_buff *msg) >>>>>> +{ >>>>>> + void *hdr = genlmsg_data(NLMSG_DATA(msg->data)); /* XXX: nlh is >>>>>> right at the start of msg */ >>>>>> + genlmsg_cancel(msg, hdr); >>>>>> + nlmsg_free(msg); >>>>>> + return -ENOBUFS; >>>>>> +} >>>>> >>>>> This seems weird. >>>> >>>> Why? >>> >>> Because it's strange to cancel then free? >> >>> From looking at the other code, the usual pattern for netlink is >> >> (please, correct me if I'm wrong): >> >> int fill (....) >> { >> genlmsg_put(); >> NLA_PUT(...) >> NLA_PUT(...) >> return genlmsg_end(); >> >> nla_put_failure: >> genlmsg_cancel(); >> return -EMSGSIZE; >> } >> >> int cmd(...) >> { >> int rc; >> nlmsg_new(); >> >> rc = fill(); >> if (rc < 0) >> goto out; >> >> genlmsg_unicast(); >> >> out: >> nlmsg_free(); >> return -ENOBUFS; >> } >> >> This is equivalent to our code: >> >> sk_buff *new() >> { >> msg = nlmsg_new(); >> genlmsg_put(); >> return msg; >> } > > canceling messages is only necessary in fill functions that are > also used for dumps, where as many elements are stuffed in the > skb as possible. When the last element doesn't completely fit, > its removed again (canceled) and put into the next skb. Understood. So you'd suggest just to drop the genlmsg_cancel() from the failure call chain? -- With best wishes Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html