* 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