Garbage value when swapping array with memcpy()

Question

I am trying to make a generic swap function using memcpy() in C. I am getting a garbage value when I try to swap arrays.

This is code below:

#include<stdio.h>
#include<string.h>

typedef struct Student
{
    char a[10];
    int b;
    double c;
}   Student;

void swap(void* one, void* two,size_t size)

{
    char temp[size] ;
    memcpy(temp,two,size); // temp = *two;
    memcpy(two,one,size);//*two = *one;
    memcpy(one,temp,size);//*one = temp;
}

int main()
{
    int i1 = 10, i2 = 20;
    float f1 = 1.6, f2 = 8.9;
    int a1[3] = {1, 2, 3}, a2[3] = {10, 20, 30};
    Student s1 = {"Mark", 42, 5.2}, s2 = {"Bilal", 9, 3};
    swap(&i1,&i2,sizeof(int));
    printf(" i1 = %d \n i2 = %d\n",i1,i2);
    swap(&f1,&f2,sizeof(double));
    printf(" f1 = %f \n f2 = %f\n",f1,f2);
    swap(&a1,&a2,sizeof(a2));
    printf(" a1 = %d %d %d  \n a2 = %d %d %d ",a1[0],a1[1],a1[2],a2[0],a2[1],a2[2]);

}

The output is below:

I am also getting a garbage value for the array.

Answer 1

The problem is very likely

swap(&f1,&f2,sizeof(double));

The variables f1 and f2 are of type float, not double. The size of float is commonly four bytes, while the size of double is eight bytes.

Because you pass the wrong size your memcpy calls will go out of bounds and lead to undefined behavior.

Use the size of the variables instead:

swap(&f1,&f2,sizeof f1);

NB: That the value of f2 is printed as zero should be a pretty big hint that the problem lies there.

---

Also note that &a1 and &a2 is wrong. Those are pointers to the actual arrays themselves, while you should pass pointers to the first element of the array. The type of both &a1 and &a2 is int (*)[3], but you should pass int *. Instead pass e.g. &a1[0], or just plain a1 as that will decay to a pointer to its first element.

Answer 2

Please try this

#include <stdio.h>
#include <string.h>

typedef struct Student {
char a[10];
int b;
double c;
} Student;

void swap(void* one, void* two, size_t size) {
char temp[size];
memcpy(temp, one, size);
memcpy(one, two, size);
memcpy(two, temp, size);
}

int main() {
int i1 = 10, i2 = 20;
float f1 = 1.6, f2 = 8.9;
int a1[3] = {1, 2, 3}, a2[3] = {10, 20, 30};
Student s1 = {"Mark", 42, 5.2}, s2 = {"Bilal", 9, 3};

swap(&i1, &i2, sizeof(int));
printf("i1 = %d\ni2 = %d\n", i1, i2);

swap(&f1, &f2, sizeof(float));
printf("f1 = %f\nf2 = %f\n", f1, f2);

for (int i = 0; i < 3; i++)
    swap(&a1[i], &a2[i], sizeof(int));

printf("a1 = %d %d %d\na2 = %d %d %d\n", a1[0], a1[1], a1[2], 
a2[0], 
a2[1], a2[2]);

return 0;
}

Explanation:

The swap function remains the same. We are still using memcpy to swap the memory of two variables, but now it's properly aligned with their respective data types.

The issue with the float values is fixed by passing the correct size of float (use sizeof(float)) to the swap function.

The array swap inside the loop is also fixed by passing the correct size of int (use sizeof(int)) to the swap function.

After making these corrections, the code should work as expected without any garbage value issues.

Answer 3

Floats are 4 bytes, while doubles are 8 bytes. The code got confused and overwrote memory (a hazard of memcpy()).

See below:

int main( void )
{
    int i1 = 10, i2 = 20; // Integers
    swap( &i1, &i2, sizeof i1 );
    printf(" i1 = %d \n i2 = %d\n",i1,i2);

    float f1 = 1.6, f2 = 8.9; // Floats
    swap( &f1, &f2, sizeof f1 ); // NOT sizeof "double"!!
    printf(" f1 = %f \n f2 = %f\n",f1,f2);

    int a1[3] = {1, 2, 3}, a2[3] = {10, 20, 30}; // arrays
    swap( a1, a2, sizeof a1 );
    printf(" a1 = %d %d %d  \n a2 = %d %d %d ",a1[0],a1[1],a1[2],a2[0],a2[1],a2[2]);

    Student s1 = {"Mark", 42, 5.2}, s2 = {"Bilal", 9, 3};
    // and so on...
}

Ask yourself why you entangled all the definitions without printing each as it appeared in the code.

Further, by using the sizeof() the destination variable, you are less likely to overrun the destination buffer, even if the source is incorrect.

Answer 4

swap(&f1,&f2,sizeof(double));

It is one of the classic examples why one should not use types in sizeof.

swap(&f1,&f2,sizeof(f1));

solves the problem for any f1 type.