On Thu, Nov 10, 2011 at 6:12 PM, Andrew Deason <[email protected]> wrote:
> On Tue, 19 Jul 2011 18:10:38 -0500
> Tom Keiser <[email protected]> wrote:
>
>> Please see
>> http://tools.ietf.org/html/draft-keiser-afs3-xdr-primitive-types-00.
>> Comments are hereby solicited.
>
> Comments:
>
>>> 1. Introduction
> [...]
>>> The Rx RPC protocol utilizes XDR [RFC4506] as its means of encoding
>>> RPC call and response payloads. While XDR provides type definitions
>>> for each popular bit-length integer, it does so using relatively
>>> ambiguous names (e.g., hyper).
>
Hi Andrew,
Thank you for the detailed review.
> I don't think it's quite right to call the names "ambiguous", as the
> format, width, etc are explicitly defined in RFC4506. I think
> "confusing" makes more sense. I don't think this is deserving of a new
> revision or anything, though; but if you happen to cut a new one for
> other reasons anyway...
>
That's fair. I suspect this will go through at least one more
revision, so I will adopt your wording.
>>> 4. afsUUID
> [...]
>>> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>> | {3} | node[4] |
>>> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>> | {3} | node[5] |
>>> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>
>>> {1} clock_seq_hi_and_reserved
>>> {2} clock_seq_low
>>> {3} sign extended padding: 0, or 0xffffff depending on value
>>> of MSB in field to be sign-extended and padded
>
> Should there be some kind of explicit identification of which {3} fields
> correspond to the sign extension of which field? This is pretty
> intuitive by the rest of the document, but it is not explicit in this
> diagram.
>
> However, I think this separation unnecessarily complicates the spec; the
> diagram layout implicitly depends on XDR "internals" (that is, node[5]
> is on the right because XDR encodes with NBO, but we don't care since we
> offload the encoding to the XDR "int" type). I think it would be easier
> to just, for example, have the whole 32-bit row be node[5], but you just
> restrict node[5] to having values between 127 and -128 inclusive.
>
Ok. I'll simplify this to just be a set of integer range constraints,
and otherwise discuss these fields as being sent over the wire as
32-bit wide XDR integers.
> See below:
>
>>> 4.1. Encoding
> [...]
>>> Many of the fields which are encoded in this data structure are
>>> smaller than four octets. In order to XDR encode these fields, they
>>> must first be resized. Since many of these fields are signed, this
>>> involves sign extension. This process depends upon the machine
>>> architecture. Virtually all machines in existence today utilize 2s-
>>> complement integer arithmetic, where this process merely involves
>>> padding with zeros if the MSB is zero or ones if the MSB is one.
>
> This seems to suggest that the encoding and decoding depends on the
> in-memory implementation of negative numbers of the machine doing the
> encoding and decoding, which is definitely inappropriate for a network
> protocol. I find this section a bit confusing, though; I don't see the
> need to really discuss sign-extension of the values, since the encoding
> of the individual fields is dictated by XDR. That is, if node[0] is -5,
> the algorithm for encoding is not "sign-extend -5 according to
> 2s-complement", but rather "give the XDR encoding routine a 32-bit
> integer of value -5".
>
Hmm. That's a good point. Dealing with how the microarchitecture
represents the integer is purely XDR's problem; I'll simplify the
language and the diagram to follow this line of reasoning.
Cheers,
-Tom
_______________________________________________
AFS3-standardization mailing list
[email protected]
http://lists.openafs.org/mailman/listinfo/afs3-standardization
