HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Byte swapping functions

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
bytefunctionsswapping

Problem

I recently programmed some byte swapping functions. As a special I did a function which can convert an entire int array. I please you to look at the following aspects:

  • portability



  • performance



  • usability



  • efficiency



#include

uint16_t _bswap16(uint16_t a)
{
a = ((a & 0x00FF) > 8);
return a;
}

uint32_t _bswap32(uint32_t a)
{
a = ((a & 0x000000FF) > 8) |
((a & 0xFF000000) >> 24);
return a;
}

uint64_t _bswap64(uint64_t a)
{
a = ((a & 0x00000000000000FFULL) > 8) |
((a & 0x0000FF0000000000ULL) >> 24) |
((a & 0x00FF000000000000ULL) >> 40) |
((a & 0xFF00000000000000ULL) >> 56);
return a;
}


Byte Swapping functions for arrays:

#include
#include
#include
#include

void _bswap_a(void restrict const array, size_t const elem_size, size_t const ndimensions, size_t const restrict const dimensions, bool const copy, void (bswapfcn)(void *))
{
/ Validate arguments /
if (array == NULL || elem_size

Usage:

int main(int argc, const char **argv)
{
/ Swap 32bit Integer /
uint32_t _swappedInteger = _bswap32(32);
/ Swap 32bit Integer array (2 dimensional) and copy the array /
uint32_t _2darray[2][4] = {{8, 7, 6, 5}, {4, 3, 2, 1}};
size_t dimensions[2] = {2, 4};
uint32_t (_swappedArray)[4] = (void )_bswap32a((void *)_2darray, 2, dimensions, true);

fprintf(stderr, "%-12u:%4u \n", _swappedInteger, _bswap32(_swappedInteger));
fprintf(stderr, "%-12u:%4u \n", _swappedArray[1][2], _bswap32(_swappedArray[1][2]));

/ _swappedArray is a copy of _2darray so we need to free it /
free(_swappedArray);
}
`

Solution

What's with all the underscores? Names starting _ are very unpleasant to use in client code. This naming convention is usually associated with library internals. And even so, doesn't seem very necessary.

You could very well return directly instead of assigning the result to a variable:

return ((a & 0x00FF) > 8);


That local scope inside _bswap_a is unusual and seems unnecessary:

{
    size_t array_size       = elem_size;
    size_t i                = 0;
    ....
}


Other functions that take pointers, like _bswap16f and _bswap32f, are not validating the inputs before dereferencing them. You should at least assert that the pointers are not null.

Code Snippets

return ((a & 0x00FF) << 8) | ((a & 0xFF00) >> 8);
{
    size_t array_size       = elem_size;
    size_t i                = 0;
    ....
}

Context

StackExchange Code Review Q#64797, answer score: 3

Revisions (0)

No revisions yet.