On Wed, Jul 30, 2014 at 02:14:44PM +0200, u-codalist-z149_at_aetey.se wrote:
> Looking at two very similar code snippets, like cut-n-paste
> but yet different:
> 
> venusvol.cc:
>                 /* Make multiple copies of the IN/OUT and OUT parameters. */
>                 int ph_ix; unsigned long ph;
>                 ph = m->GetPrimaryHost(&ph_ix);
> 
> fso_cfscalls0.cc:
>             /* Make multiple copies of the IN/OUT and OUT parameters. */
>             int ph_ix; unsigned long ph;
>             ph = ntohl(m->GetPrimaryHost(&ph_ix)->s_addr);


Line numbers would be nice, because I am not sure what you are looking
at,

    $ git grep GetPrimaryHost
    fso_cfscalls0.cc:            ph = ntohl(m->GetPrimaryHost(&ph_ix)->s_addr);
    fso_cfscalls0.cc:            ph = ntohl(m->GetPrimaryHost(&ph_ix)->s_addr);
    fso_cfscalls0.cc:            ph = ntohl(m->GetPrimaryHost(&ph_ix)->s_addr);
    mgrp.cc:struct in_addr *mgrpent::GetPrimaryHost(int *ph_ixp)
    mgrp.cc:    CHOKE("mgrpent::GetPrimaryHost: ph (%x) not found", rocc.primaryhost);
    mgrp.h:    struct in_addr *GetPrimaryHost(int *ph_ixp=NULL);
    venusvol.cc:                ph = ntohl(m->GetPrimaryHost(&ph_ix)->s_addr);
    venusvol.cc:                ph = ntohl(m->GetPrimaryHost(&ph_ix)->s_addr);
    venusvol.cc:            ph = ntohl(m->GetPrimaryHost(&ph_ix)->s_addr);
    vol_cml.cc:        phost = m->GetPrimaryHost(&ph_ix);
    vol_cml.cc:    phost = *m->GetPrimaryHost(&ph_ix);
    vol_resolve.cc:     struct in_addr *phost = m->GetPrimaryHost();

So the only ones I see that do not use ntohl are the calls in vol_cml
and vol_resolve.

And there is a good reason for that, in fso_cfscalls/mgrp/venusvol it
is passed as an RPC2 argument, which remaps from client to server byte
order, so we have to canonicalize first otherwise the address might get
end up in reverse order on the server. Another solution would be to add
an ipaddress RPC2 datatype that knows how to properly deal with
endianess and ipv4/ipv6 issues instead of stuffing a v4 address into a
32-bit 'rpc2_integer'.

The ones in vol_cml and vol_resolve are used to get the ip address of
the primary server, which is the server that has the best
latency/bandwidth based on historical estimates, so which is likely the
least-loaded/closest server to reintegrate to.

> I wonder whether both can be correct and whether one of the cases
> is compensated by the corresponding miss on the server side
> and will break for different endianness of a client and a server?

Yes both are correct, different uses. The non-converted one is only used
client-side, the one that is converted is shipped off to the server and
was used at some point in Coda's past to reduce the number of callbacks
that were kept. Only the 'primary host' replica used to track client
callbacks, now all replicas do.

Jan