rxgk CombineTokens and enctypes (was Re: [AFS3-std] Re: afs3-rxgk-updates for 03)

[Benjamin Kaduk]

On Sat, 3 Nov 2012, Benjamin Kaduk wrote:

> On Sat, 3 Nov 2012, Simon Wilkinson wrote:
>
>>>> 1) Whichever one the server picks from the client-provided list.
>>> 
>>> Okay.  I would pick (1) from that, and prefer that the client indicates 
>>> its preferences.
>> 
>> If we're doing this (and I have no objection to doing so, as it's
>
> I think this is the least bad way to get to a fully/usefully specified 
> CombineTokens operation.  (That is, I think we should do it.)
>
>> necessary for AFSCombineTokens, in any case) then we need to change the 
>> signature of CombineTokens to something like:
>> 
>> struct RXGK_CombineOptions {
>>    RXGK_Enctypes enctypes;
>>    RXGK_Levels levels;
>
> I don't think we have an array RXGK_Levels typedef in the document at the 
> moment, and adding one in addition to the non-plural RXGK_Level enum might be 
> confusing.  So I would put this as 'RXGK_Level levels<>' to match 
> StartParams.  (Sorry, nitpicking.)

I'm actually going to have to reverse myself here, and advocate for an 
RXGK_Levels typedef.

It turns out that when using a variable-length array inside a structure, 
rxgen declares locally a struct type with the struct name as the name of 
the variable that is the variable-length array.  E.g.:

struct RXGK_ClientInfo {
     opaque token<>;

becomes

struct RXGK_ClientInfo {
         struct token {
                 u_int token_len;
                 char *token_val;
         } token;

When we have the same name appear as a struct member of variable length in 
two different protocol structures, the generated header file from rxgen 
does not compile, because this local struct is redefined.  We see this 
with RXGK_Level levels<> in StartParams and CombineOptions, and with 
token<> in ClientInfo and Response.

We could rename the member variables to work around, of course, but I 
think I prefer to add the new types RXGK_Levels and RXGK_Token.  I'd also 
add an RXGK_enctype typedef for use in the TokenInfo (and ClientInfo) 
struct, instead of hardcoding int.  (My motivation for not adding this 
type earlier was the same as in the quoted text above, the concern with 
having types differing by one character.)

-Ben