Omn
Omn

Reputation: 39

Strange behaviour then swapping values inside pointers

I have this code which is supposed to swap two values inside a_ptr and b_ptr:

// swaps two values inside two variables of any type
void swap(void *a_ptr, void *b_ptr)
{
    size_t size = sizeof(void *);
    void *tmp = malloc(size);
    memcpy(tmp, a_ptr, size);
    memcpy(a_ptr, b_ptr, size);
    memcpy(b_ptr, tmp, size);
    free(tmp);

    // char wot0 = 0;
    // char wot1 = 0;
    // swap(&wot0, &wot1);
}

int main(){

    long a = -7;
    long b = 99999999;
    printf("A: %ld, B: %ld\n", a, b);
    swap(&a, &b);
    printf("A: %ld, B: %ld\n", a, b);
    printf("\n");

    short c = -9;
    short d = 11111;
    printf("C: %hd, D: %hd\n", c, d);
    swap(&c, &d);
    printf("C: %hd, D: %hd\n", c, d);
    printf("\n");

    char ca = 'a';
    char cx = 'x';
    printf("CA: %c  CX: %c\n", ca, cx);
    swap(&ca, &cx);
    printf("CA: %d  CX: %c\n", ca, cx);
    printf("\n");

    char *str0 = "Hello, ";
    char *str1 = "World!";
    printf("STR0: %s  STR1: %s\n", str0, str1);
    swap(&str0, &str1);
    printf("STR0: %s  STR1: %s\n", str0, str1);
    printf("\n");

    return 0;
}

However the output is:

A: -7, B: 99999999
A: 99999999, B: -7

C: -9, D: 11111
C: -7, D: -9

CA: a  CX: x
CA: -9  CX: a

STR0: Hello,   STR1: World!
STR0: World!  STR1: Hello, 

It successfully swaps a and b, and then somehow replaces c with b, and ca with d, how's that even possible?

Also, uncommenting these lines:

    // char wot0 = 0;
    // char wot1 = 0;
    // swap(&wot0, &wot1);

Leads to a segfault, why?

EDIT: I think I didin't convey my intentions very well. That I basically want to do is swap pointers so that a_ptr points to value inside b, and b_ptr points to value inside a, I don't want to actually copy the values themselves and I think I achieved that somewhat successfully, strings of different lengths (for examples "Foo" and "Hello, World!") get swapped without any issues, I tested that, however I don't understand why some variables don't get swapped and actually point to values outside of that I passed into the function

Upvotes: 0

Views: 92

Answers (1)

Ted Lyngmo
Ted Lyngmo

Reputation: 117298

sizeof(void *); is a constant (usually 4 or 8) and does not give you the size of the object it's pointing at. When you copy size bytes, you are not copying the correct amount for the types used.

You're probably better off by supplying the size of the type to the function:

// swaps two values inside two variables of any type
void swapper(void *a_ptr, void *b_ptr, size_t size)
{
    void *tmp = malloc(size);
    memcpy(tmp, a_ptr, size);
    memcpy(a_ptr, b_ptr, size);
    memcpy(b_ptr, tmp, size);
    free(tmp);
}

// Generate compilation error if objects of different sizes are used.
// The false switch case (0) can only be defined once so if the sizes
// are not the same, it'll try to redefine "case 0" and fail compiling:
#define compile_assert(_expr) switch (_expr) { case 0: break; case _expr: break; } 

#define swap(x,y) do { \
    compile_assert(sizeof(*(x)) == sizeof(*(y))); \
    swapper((x),(y),sizeof(*(x))); } while (0)

And call it like you aimed to do:

swap(&a, &b);

If you only need to swap fundamental types, you could make different implementations for all of them. This should also make it safer since it's harder to supply pointers to objects of different types this way:

#define SwapBuilder(name,type) \
    void name(type *a, type *b) { type tmp = *a; *a = *b; *b = tmp; }
SwapBuilder(swap_char,char)
SwapBuilder(swap_schar,signed char)
SwapBuilder(swap_uchar,unsigned char)
SwapBuilder(swap_short,short)
SwapBuilder(swap_ushort,unsigned short)
SwapBuilder(swap_int,int)
SwapBuilder(swap_uint,unsigned int)
SwapBuilder(swap_long,long)
SwapBuilder(swap_ulong,unsigned long)
SwapBuilder(swap_longlong,long long)
SwapBuilder(swap_ulonglong,unsigned long long)
SwapBuilder(swap_float,float)
SwapBuilder(swap_double,double)
SwapBuilder(swap_longdouble,long double)

// A _Generic to call the correct function:
#define swap(x,y) _Generic((x), \
    char* : swap_char, \
    signed char* : swap_schar, \
    unsigned char* : swap_uchar, \
    short* : swap_short, \
    unsigned short* : swap_ushort, \
    int* : swap_int, \
    unsigned int* : swap_uint, \
    long* : swap_long, \
    unsigned long* : swap_ulong, \
    long long* : swap_longlong, \
    unsigned long long* : swap_ulonglong, \
    float* : swap_float, \
    double* : swap_double, \
    long double* : swap_longdouble \
)((x),(y))

And you'd still call it with swap(&a, &b);

Upvotes: 3

Related Questions