Re: [PATCH v3 3/6] nfs-utils: Implement srcaddr binding in rpc_socket

[Ben Greear <greearb@candelatech.com>]

On 06/10/2011 03:37 PM, Chuck Lever wrote:
>
> On Jun 10, 2011, at 6:19 PM, Ben Greear wrote:
>
>> On 06/10/2011 03:06 PM, Chuck Lever wrote:
>>>
>>> On Jun 10, 2011, at 5:08 PM, greearb@candelatech.com wrote:
>>>
>>>> From: Ben Greear<greearb@candelatech.com>
>>>>
>>>> This implements the actual binding, if we are passed
>>>> a non-null local_ip structure.
>>>
>>> Why not _always_ pass a valid local_ip structure, and simply set .addr to an appropriate ANYADDR by default?  Then .is_set wouldn't be necessary, would it?  It would also simplify the logic in nfs_validate_options().
>>
>> I like it as is because almost none of the new code is actually
>> used unless users pass in the srcaddr= option.  So, if I *did*
>> introduce any bugs, hopefully they would be limited to users
>> of the new option, and not a real regression.
>
> It should be pretty obvious if something here breaks.
>
>> Maybe after the srcaddr= code is used a bit I could go back and
>> do that cleanup.
>>
>> But, I don't feel strongly about it, so if you think it's
>> worth the bother, I'll try changing the code as you suggest.
>
> In the long-term, if my suggestion works out, this code would be simpler, and to me that's better than the risk of a little short-term instability.

Do you mean always make sure it is not NULL as well?

That would complicate code everywhere I currently pass NULL in for local_ip
(like in methods that don't care about binding, non stropts logic, etc).
I think that would cause more harm than good.

I could add return value to the parse method instead of relying on
is_set (and just pass in NULL instead of &local_ip if we didn't have the srcaddr=
option) if you think that is cleaner, but I don't think it will simplify things very
much...

Thanks,
Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com