Swapping two variable using pointers

Question

I am trying to write a swap function using pointer(specially a void pointer)by-reference, but my code is not working. Here is my code:

void swap(void *p1,void *p2) 
{
    int temp;   
    temp=*((int*)p2);
    p2=p1; 
    p1=&temp;
}
int main() 
{
    int i=4;
    int j=5; 
    cout<<i<<j<<endl;
    swap(&i,&j); 
    cout<<i<<j<<endl;
    return 0;
}

Where am I going wrong?

Solution 1

The code does not work because you are not dereferencing your pointers on assignments. It should be

*((int*)p2)=*((int*)p1);
*((int*)p1)=temp;

Note that you are making an assumption that void* points to an int, which is obviously not always the case. Essentially, you might as well replace void* with int*, and get rid of the casts.

A more general case of the API should look like this:

void swap(void *p1,void *p2, size_t sz)

Internally, the API should allocate a buffer of size sz, make a memcpy into it, and then make a swap, again using memcpy.

Solution 2

In the body of the function, you're swapping the values of p1 and p2; you don't want to do that. You want to swap the values of what p1 and p2 point to:

void swap(int *p1, int *p2)
{
  int tmp = *p1;
  *p1 = *p2;
  *p2 = tmp;
}

I know you wanted to use void * for your arguments. Don't. You'd have to cast them to the appropriate target type to perform the assignments anyway:

int tmp = *(int *) p1;
*(int *) p1 = *(int *) p2;
*(int *) p2 = tmp;

Yuck. You're not saving yourself anything by making the arguments void *.

Since you're obviously writing C++, you can make the function generic by using a template:

template<typename T>
void swap(T *p1, T *p2)
{
  T tmp = *p1;
  *p1 = *p2;
  *p2 = tmp;
}

Even better, use a template and references, so you're not dealing with pointers at all:

template<typename T>
void swap(T &p1, T &p2)
{
  T tmp = p1;
  p1 = p2;
  p2 = tmp;
}

Comments

[amit]

This is broken in a lot of ways, but the most important issue (IMO) is assigning p1,p2 with values, while you actually need to assign the address they address to. This is not the only issue, just the most important IMO.

[andre]

Do they need to pass the pointers by reference ?

[Rontogiannis Aristofanis]

Since you are swaping ints, you needn't pass the function parameters as voids.

[R. Martinho Fernandes]

This is assuming the types are trivially copyable-.

[Sergey Kalinichenko]

@R.MartinhoFernandes Right -- essentially, this amounts to a trivial swap of memory contents in non-overlapping regions.

[Bo Persson]

Wait a minute - haven't I seen that last function somewhere?

[MSalters]

Actually, in C++ we use template <typename T> when we don't know the type yet. The compiler will know, use the real type T and then check for errors. That's why we have template <typename T> std::swap(T&, T&)

[Guillaume Racicot]

This hardly help OP. Can you provide an example of how the "memory functions" work?