[haiku-commits] Re: BRANCH pdziepak-github.nfs4 - src/add-ons/kernel/file_systems/nfs4

  • From: Pawel Dziepak <pdziepak@xxxxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Thu, 12 Jul 2012 19:35:51 +0200

2012/7/12 Axel Dörfler <axeld@xxxxxxxxxxxxxxxx>:
> On 12.07.2012 00:49, pdziepak-github.nfs4 wrote:
>>   bool
>>   ServerAddress::operator==(const ServerAddress& address)
>>   {
>> -       return fAddress == address.fAddress && fPort == address.fPort
>> -                       && fProtocol == address.fProtocol;
>
>
> Out of curiosity: what are the fPort/fProtocol fields used for?

ServerAddress is used to identify a RPCServer object when a NFS4 file
system is mounted (one connection per server, doesn't matter how many
file systems are mounted). Since NFS4 supports both TCP and UDP the
client needs to know which transport protocol RPCServer object uses.
fPort was removed in commit 137884e.

>> +       switch (reinterpret_cast<const
>> sockaddr*>(&fServerAddress)->sa_family) {
>> +               case AF_INET:
>> +                       addressSize = sizeof(sockaddr_in);
>> +                       break;
>> +               case AF_INET6:
>> +                       addressSize = sizeof(sockaddr_in6);
>> +                       break;
>> +               default:
>> +                       return B_BAD_VALUE;
>> +       }
>
>
> Maybe move that code into a ServerAddress::Size() method? Besides that,
> getsockname() shouldn't mind if the size given is larger than the size it
> needs to store the information (although that part is not portable, as other
> platforms can be strict about this).

According to [1] getsockname() is not allowed to cause problems when
the given size is too big. I should have check that earlier.
I will add ServerAddress::Size() as well as some accessors (for
example ServerAddress::Port()) since ServerAddress implementation is
much better place to deal with the internals of sockaddr_* structures
than, for example RequestBuilder::_GenerateClientId().

> Maybe ServerAddress could be a light-weight version mix of
> BNetworkAddress/BNetworkAddressResolver instead? And since there is nothing
> server specific about ServerAddress, the class could then be renamed, too
> :-)

As I said before ServerAddress is used as server identifier not only a
structure to store IP address. That's why it should not be considered
as kernel version of BNetworkAddress. Actually, if BNetworkAddress was
available in kernel I would make fAddress of that type. However, if
think I should rename ServerAddress to ServerID since apparently
current name is misleading.

Thanks for feedback.
Paweł

[1] http://pubs.opengroup.org/onlinepubs/009695399/functions/getsockname.html

Other related posts: