[PATCH] Some code improvements (no functional change)

[Duncan P. N. Exon Smith]

On 2014-Apr-17, at 2:49, Artyom Skrobov <Artyom.Skrobov at arm.com> wrote:

> Thank you Duncan.
> 
>> I don't think the code churn of the other include guards is worth it.
>> They're private headers that work together as is, and haven't been
>> modified since the original commit.
> 
> To be fair, regutils.h had been modified once since then (r81114); but is
> there any reason not to add the guards?

There are cases were header guards are incorrect (break functionality),
but I doubt this is one of them.  Nevertheless, I don't see any reason for
code churn for the sake of churn.

>> When the function is local to a file, SwapValue() is clear enough, but
>> once it's API:  how do the names SwapValue() and SwapByteOrder() match
>> up together?
> 
> I agree that SwapValue() is not a very good name to begin with.

I don't think the name is a big problem when it's locally defined.

> Would it be reasonable if we name both SwapByteOrder() -- it's difficult to
> describe their purpose in any other way -- and make the in-place function
> take a pointer, instead of a reference?

Pointer is the wrong API: it implies having to check for null.

> That way, it would be more evident from the call site whether the function
> operates on the argument or takes a copy.

Frankly, this is a one-liner wrapper defined locally in two places.  I don't
think this is worth doing.

However, if you're determined (or are planning to use this elsewhere),
`SwapByteOrderInPlace()` would be fine by me.

If it's in a public header, it should be declared `inline`, not `static`.