sion_swap's aflag argument has different meaning for in-place vs. disjunct buffers
SIONlib's byte swap function, sion_swap
, has the following documentation and signature:
/**
* perform byte-order swapping for arrays
* n units of size byte are swapped
* if and only if aflag==0
* in-place swapping (target==source) is allowed
* if target != source, the buffers must not overlap
*/
void sion_swap(void* target, void* source, int size, int n, int aflag);
And there seems to be some confusion about the aflag
argument. Different pieces of documentation seem to have different things to say about it:
- The documentation comment (this is also part of the Doxygen documentation) above the function definition in
sion_convert.c
was recently changed (see r2136) to invert the meaning ofaflag
from what it had been all the way since r49, namelyaflag == 1
means swap bytes. - Curiously, there is another documentation comment (this one does not go in the Doxygen output) introduced in
sion.h
(nowadays it resides insion_common.h
) all the way back in r43 which claimsaflag == 0
means swap bytes, contradicting the original comment insion_convert.c
. - The training material used in the PATC parallel I/O course in the last two years sides with the
sion_common.h
comment in sayingaflag == 0
means swap bytes. - This is not really documentation, but all internal uses use
filedesc->swapbytes
in place ofaflag
, which again supports the first interpretation,aflag == 1
means swap bytes. The Mandelbrot test supports this further, calling the variable it uses in place ofaflag
doswap
.
So which meaning is true and whence the confusion? Well, it turns out that both are true, because the meaning of aflag
changes, depending on whether target == source
. The internals of sion_swap
actually look like this:
void sion_swap(void* target, void* source, int size, int n, int aflag) {
if (target == source) {
if (!aflag) { // Note the negation operator
// do nothing
} else {
// swap in place
}
} else {
if (aflag) { // Note the absence of the negation operator
// memcpy from source to target (no swapping)
} else {
// swap bytes while copying
}
}
}
So, at the moment, all pieces of documentation do not fully describe the behavior of sion_swap
. I see three ways to fixing this:
- Make
aflag == 1
mean swap bytes in all cases and adapt all documentation. Pro: small internal code changes, since they all fall into the in-place case which already matches this. Contra: this meaning contradicts the PATC training material. - Make
aflag == 0
mean swap bytes in all cases. Pro: matches the PATC training material. Contra: more internal code changes and contradicts the Doxygen documentation. - Keep the behavior as it is and adapt the documentation to explain both cases. Pro: no code changes. Contra: convoluted logic.