* Patrick McHardy <[EMAIL PROTECTED]> 2005-08-14 15:30
> Thomas Graf wrote:
> > * Patrick McHardy <[EMAIL PROTECTED]> 2005-08-13 02:36
> > 
> >>[NETLINK]: Support dynamic number of multicast groups per netlink family
> >>
> >>Signed-off-by: Patrick McHardy <[EMAIL PROTECTED]>
> >>
> >>-   if ((err = __netlink_create(sock, protocol) < 0))
> >>+   nlk->groups = kmalloc(NLGRPSZ(groups), GFP_KERNEL);
> >>+   if (nlk->groups == NULL) {
> >>+           err = -ENOMEM;
> >>            goto out_module;
> >>+   }
> > 
> > 
> > Inteded to depute the cleanup of __netlink_create to a
> > call to sock_release() by the caller?
> 
> Sorry, I'm not sure I understand what you mean :)

You don't undo the allocations of __netlink_create if the
kmalloc for nlk->groups fails. So my question is if this is
intended and you really want to rely on the caller to invoke
sock_release() to free the sk again or whether it might be
worth to follow the rule of leave things untouched in case
of an error.

> > Given we remove the minimal group size of 32 introduced
> > in a later patch would it make sense to not allocate if
> > groups==0 at the cost of a few additional runtime checks?
> > I only see a real cost in do_one_broacast() but the
> > check for group -  1 >= ngroups already ensures it to be
> > allocated so I don't see any problems performance wise.
> 
> We could do that, the main reason why my patches enforce a minimum
> of 32 groups is for backwards compatiblity so getsockname returns the
> same nl_groups mask that was specified in bind. I'm not sure if we
> really need this ..

Good point, I'll think about this.
-
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