[AFS3-std] Re: AFSVol GetSizeV2 draft

Jeffrey Hutzelman jhutz@cmu.edu
Wed, 09 Feb 2011 12:37:43 -0500


--On Wednesday, February 09, 2011 10:08:14 AM -0600 David Boyes 
<dboyes@sinenomine.net> wrote:

>> But the "transaction" above it doesn't exist in a standard. While
>> conceptually it exists, the only place it's really documented as such
>> is
>> in the existing implementation(s). So, as I read it, giving the RPCs a
>> successful return code is already tying us to the implementation, since
>> it just comes from the convention of the C librx implementation. At
>> least, that's how I read "if you really want to avoid making
>> assumptions
>> based on the implementation..."
>
> Gotta start somewhere. Implied operations are not good practice

No.  Undocumented assumptions are not good practice.  Implied things are 
fine, as long as they're documented.


You asked about the section that defines error codes, and whether we had a 
constant to use for "success".  The answer is that we do not, because the 
error codes in question are the codes sent in an Rx abort packet when an 
RPC fails.  If the RPC succeeds, no abort is sent, and thus there's no need 
for a code to be used in abort packets to indicate success.

The question of how the Rx layer indicates to the application the 
distinction between a call that succeeds, a call that is aborted by the 
peer, and a call that fails for local reasons is entirely up to the 
implementation.




> Really, how tough is it to #define RX_SUCCESS 0 and then use the constant
> in the text instead of hardcoding a fixed value in the spec?

There is no fixed value hardcoded in the spec.  The packet that contains 
the error code _is not sent at all_ if the call doesn't fail.

OpenAFS's Rx implementation models RPC's as function calls, returning an 
integer which is the error code from the abort, or a locally-generated 
error, or 0 if the call succeeds.  For such an implementation, it is mostly 
reasonable to define a constant for success(*).  But that's up to the 
implementation; it doesn't appear on the wire and is not part of the spec.

-- Jeff



(*) Mostly, but not entirely.  Success-as-zero is _not_ just another value, 
because zero is special in C and most C-like languages.  A function which 
returns 0 on success allows for relatively clean, simple code like

if (unlink("/my/file")) {
  ... do something about the error ...
}

Whereas if unlink returns 5 on success, you have to write
if (unlink("/my/file") != 5) { ... }
or even
if (unlink("/my/file") != SYSCALL_SUCCESS) { }

and it starts to get harder to read.

If something is going to return 0 on success, then it should be defined to 
return 0, not FOO_SUCCESS-which-just-happens-to-be-zero, because doing so 
allows callers to use language features that interpret expressions in a 
boolean context, and because on some architectures, comparison to zero is 
shorter, faster, and/or implied.