Swap on void* array doesn't work correctly

Question

I have an array of complex numbers, and I just want to swap between two elements. However, I want to implement the swap for unknown types (using void*). So I wrote the following code, using the first swap implementation that I saw here:

#include<stdio.h>

typedef struct complex complex;

struct complex {
    int real;
    int img;
};

void swap(void *arr[], int i, int j)
{
    void *temp;
    temp = arr[i];
    arr[i] = arr[j];
    arr[j] = temp;
}

void printComplexArray(complex* cArr, size_t len)
{
    int i;
    for (i = 0; i < len; i++)
    {
        printf("Cell %d : real- %d , img- %d\n", i, cArr[i].real, cArr[i].img);
    }
}

void main(void)
{
    complex cArr[] = { { 22, 3 },{ 1, 13 },{ 5, 7 },{ 8, 4 } };
    size_t cLen = (sizeof(cArr) / sizeof(cArr[0]));
    swap(cArr, 1, 2);
    printComplexArray(cArr, cLen);
}

I expected to get:

Cell 0 : real- 22 , img- 3
Cell 1 : real- 5 , img- 7
Cell 2 : real- 1 , img- 13
Cell 3 : real- 8 , img- 4

But I got:

Cell 0 : real- 22 , img- 1
Cell 1 : real- 3 , img- 13
Cell 2 : real- 5 , img- 7
Cell 3 : real- 8 , img- 4

As you can see, not only that the swapping wasn't performed correctly- the content of some array elements was changed. Can you explain why, and how can I fix this?

Any reason you can't just use `complex.h` with its superior support? — StoryTeller - Unslander Monica, Mar 08 '17 at 09:25
Yes, I'm not allowed to include any h files (besides stdio.h) — Rodrigo, Mar 08 '17 at 09:29
`complex cArr[]` is not `void *arr[]`... You should look at compiler warnings... — LPs, Mar 08 '17 at 09:29
@LPs , I don't get any warning (using visual studio). Can you tell how to declare cArr correctly so that I won't have this problem? — Rodrigo, Mar 08 '17 at 09:37

Jabberwocky · Answer 1 · 2017-03-08T10:34:05.513

6

You probably want this:

void swap(struct complex arr[], int i, int j)
{
    struct complex temp;
    temp = arr[i];
    arr[i] = arr[j];
    arr[j] = temp;
}

Your solution swaps pointers but your array doesn't contain pointers but struct complexs. That's basically what the compiler warning told you.

If you really want to swap unknown types, you need this:

void swap(void *arr, int i, int j, size_t size)
{
    char temp[size];
    char *a = (char*)arr;

    memcpy(temp, (a + size * i), size);
    memcpy((a + size * i), (a + size * j), size);
    memcpy((a + size * j), temp, size);
}
...
swap(cArr, 1, 2, sizeof(*cArr));

The size parameter is necessary because if the type is unknown, the size of the type is of course unknown too and therefore you need to specify the size.

For memcpy you need to include <memory.h>, so you must write your own memcpy, as you are allowed to include only <stdio.h>.

Some compilers won't allow char temp[size]; so you just can use char temp[MAX_SIZE]; with MAX_SIZE defined to the maximum expected size for your unknown types.

edited Mar 08 '17 at 10:34

answered Mar 08 '17 at 09:34

Jabberwocky

48,281
17
65
115

I don't get any warning (using visual studio). Anyway, I must use void* in my swap. Can you tell how can I change my code so that I will be able to use swap with void* arr? – Rodrigo Mar 08 '17 at 09:36
1

@Rodrigo Read the last sentence of this answer. Your array doesn't contain pointers. Therefore there are no `void*` to swap. This answer is correct for the task you've posted, so either your instruction isn't clear or was misunderstood. – WhozCraig Mar 08 '17 at 09:44
1

Should be `swap(cArr, 1, 2, sizeof( * cArr));` (size of single element, not whole array); alternative to tmp[MAX_SIZE] (if no maximum imposition is desired) might be using malloc/free or - where available - even alloca. – Aconcagua Mar 08 '17 at 10:23
@Aconcagua thanks for the `sizeof( * cArr)`. Using `malloc/free` is possible, but expensive, and `alloca` is deprecated but inexpensive. – Jabberwocky Mar 08 '17 at 10:35
1

@MichaelWalz Well, the C99's variable length array `char tmp[size]` certainly is best option anyway - just wanted to give hints to alternatives not imposing size limits (and possibly allocating far too large memory in other cases, btw...). Should have mentioned the disadvantages myself. Hm, alloca is not documented as deprecated, at least on the linux sites I looked at (MSVC does so, though). If it actually is, then probably in favour to exactly these variable length arrays... – Aconcagua Mar 08 '17 at 12:18

score 1 · Answer 2 · answered Mar 08 '17 at 09:52

You can't implement swap for unknown type, You need to know the size of the type. Your function swaps pointers (pointers have known size), but You are feeding it array of complex, not an array of pointers. Try this (not tested):

void swap(void *array, size_t i, size_t j, size_t element_length) {

    char *src, *dst, tmp;

    src = (char*)array + i * element_length;
    dst = (char*)array + j * element_length;

    while (element_length--) {

        tmp = *src;
        *src = *dst;
        *dst = tmp;

        src++;
        dst++;
    }
}

...

swap(cArr, 1, 2, sizeof(complex));

score 1 · Answer 3 · answered Mar 08 '17 at 09:53

The only way to do so using void * (as far as I can see) is to re-design your swap function swapping/coping byte of structs, like

#include<stdio.h>

typedef struct complex complex;

struct complex {
    int real;
    int img;
};


void swap(void * to, void * from, size_t len)
{
    unsigned char *uc_to   = to;
    unsigned char *uc_from = from;
    unsigned char tmp;

    for (size_t i = 0; i < len; i++)
    {
        tmp = uc_to[i];
        uc_to[i] = uc_from[i];
        uc_from[i] = tmp;
    }
}

void printComplexArray(complex* cArr, size_t len)
{
    size_t i;
    for (i = 0; i < len; i++)
    {
        printf("Cell %zu : real- %02d , img- %02d\n", i, cArr[i].real, cArr[i].img);
    }
}

int main(void)
{
    complex cArr[] = { { 22, 3 }, { 1, 13 }, { 5, 7 }, { 8, 4 } };

    size_t cLen = (sizeof(cArr) / sizeof(cArr[0]));

    swap((void *)(&cArr[0]), (void *)(&cArr[1]), sizeof(complex));

    printComplexArray(cArr, cLen);
}

Swap on void* array doesn't work correctly

3 Answers3

Linked