[AFS3-std] Comments on draft-keiser-afs3-xdr-union-06

Benjamin Kaduk kaduk@MIT.EDU
Mon, 7 Apr 2014 14:47:58 -0400 (EDT)


On Sat, 5 Apr 2014, Simon Wilkinson wrote:

> Hi,
>
> As an extension to RFC4506 this document seems fine. One nit, however is that our current rxgen doesn't implement the 4506 syntax for unions.
>
> A RFC4506 style union definition is:
>
> union switch (DESC-TYPE DESC-NAME) {
>    ...
> } NAME;

If I am reading it correctly (e.g., section 4.18), this is the RFC4506 way 
to declare and name a single instance of a union, at the location where it 
appears...

> However, AFS's rxgen takes a union defintion in the form
>
> union NAME switch (DESC-TYPE DESC-NAME) {
>    ...
> };

... whereas this would be the RFC 4506 syntax for typdef-ing the name 
'NAME' to refer to the union type defined here.

It looks like the OpenAFS codebase's .xg files do not use enum, only union
and struct, but only in the "implicit typedef" form.  We do not always use 
the typedef'd type name when putting structs in other structs, which 
perhaps adds some confusion ("struct AFSFid netFid" and "AFSFid netFid" 
would be equivalent as members of some other struct).

> For an extended union, this document defines
>
> ext-union switch (DESC-TYPE DESC-NAME) {
>    ...
> } NAME;

This draft prefixes the example as "Extensible discriminated unions are 
defined in RPC-L as follows:", which could be interpreted to mean the 
"declare-and-name-a-single-instance-where-it-appears" case, and the 
implicit typedef case is, well, implicit.

> So we're consistent with RFC4506, but inconsistent with the syntax used 
> in deployed AFS protocol definitions. I don't know to what extent this 
> is an issue, but it would seem difficult if we're moving towards a style 
> of XDR that can't be compiled by the current protocol compilers.

I'm not actually very familiar with either the OpenAFS rxgen or RC 4506 
XDR, but it seems like maybe it is just the case that OpenAFS rxgen does 
not handle unions in the non-implicit-typedef case?

Perhaps I am just confused.

-Ben