Re: [PATCH 5/6] bond: Implement a basic set of rtnl link ops

[ebiederm@xmission.com (Eric W. Biederman)]

Patrick McHardy <kaber@trash.net> writes:

> Eric W. Biederman wrote:
>> Patrick McHardy <kaber@trash.net> writes:
>> 
>>> Eric W. Biederman wrote:
>>>> +static struct rtnl_link_ops bond_link_ops __read_mostly = {
>>>> +	.kind		= "bond",
>>>> +	.setup		= bond_setup,
>>>> +	.validate	= bond_validate,
>>>> +};
>>> One more thing - you need to initialize .priv_size here so
>>> the devices created through rtnl_link have enough private
>>> room allocated.
>> 
>> Wow and the code works when I test it without that ouch!
>> 
>> As for rtnl_link_register it always succeeds so let's just
>> remove the return code and call it good.
>
> You need unroll anyways for the other failure conditions, so
> why not simply add an err1/2 and be safe for future changes?

Not a real problem.  I was just thinking of things like the
dummy driver that have this same issue and the fact that since
rtnl_link_register never fails we never test the error path.
So it would be much less error prone and less code to remove
the possibility of rtnl_link_register failing.

Eric