Your code works – as far as I can see – correctly now. But I would rewrite it to improve the readability (and with it the maintainability) of the code.
First, the repeated cast (char*)object
can be avoided by defining
a variable
char *ptr = objects;
once. Next, the address calculations can be simplified by separating the swapping of two objects into a helper function:
void reverse(void *objects, int size, int length) {
char *ptr = objects;
for (int i = 0, j = length - 1; i < j; i++, j--) {
swapObjects(ptr + size * i, ptr + size * j, size);
}
}
where swapObjects()
is:
static inline void swapObjects(char *o1, char *o2, int size) {
for (int c = 0; c < size; c++, o1++, o2++) {
*o1 = *o1 ^ *o2;
*o2 = *o1 ^ *o2;
*o1 = *o1 ^ *o2;
}
}
However, I can see no advantage in this "magically swap two bytes without temporary storage" and would rewrite this simply as
static inline void swapObjects(char *o1, char *o2, int size) {
for (int c = 0; c < size; c++, o1++, o2++) {
char tmp = *o1;
*o1 = *o2;
*o2 = tmp;
}
}
which is much simpler (no XOR operations and less memory reads and
writes). (It is possible to make the swap operation faster by using
int
or long
as temporary storage, but this requires a careful
check of the sizes and memory alignments.)
In addition, I would use size_t
as type for the size
and length
parameters. size_t
is the correct type to describe the size
of an object in memory, see for example the declarations
of malloc()
and calloc()
.
- 24.2k
- 2
- 38
- 96