[OpenAFS-devel] Proposal for capabilities support in Unix client 1.4.x

[Jeffrey Hutzelman]

--On Wednesday, June 17, 2009 09:29:02 AM -0400 Jeffrey Altman 
<jaltman@secure-endpoints.com> wrote:

> Felix Frank wrote:
>
>> The way I'm currently freeing Capabilities as allocated by the client
>> stub strikes me as (while conceptionally sound) spectacularly wrong -
>> how is this supposed to be done?
>> For a simple case as this (one lone pointer that needs freeing),
>> osi_free() appears to be the most sensible thing to use after all, as
>> that is what xdr ultimately does.
>
> Frank:
>
> Call xdr_free() to deallocate memory allocated by the XDR package.

Yes, or better, use XDR in "free" mode, which saves you from having to know 
what is supposed to be freed and what isn't.  But yeah, just calling 
xdr_free() on the ->val thing here should be fine.

>   xdr_free(caps, j * sizeof(Capabilities));

This is a bad example; in the patch in question, caps is the array of 
Capabilities structures allocated for rx_multi.  It was allocated using 
afs_osi_Alloc(), not by XDR, and so should be freed using afs_osi_Free(), 
as Frank's patch does.


> As for the afs_setTime piece.  As you and Simon discussed, there is only
> one host 'afs_setTimeHost' whose time is used to set the local time.
> I believe it is a waste of effort to call GetTime on every host.  You
> just need to make sure that GetTime is called on 'afs_setTimeHost' if
> 'afs_setTimeHost' is non-NULL or at least one of the servers such that
> afs_IsPrimaryCell(sa->server->cell) is TRUE.

Agree.  GetTime should be called only against the set-time host or if 
GetCapabilities is unsupported (and really, you can punt the latter case -- 
getting RXGEN_OPCODE from GetCapabilities is enough to tell you the server 
is up, which is all you really care about).

-- Jeff