2

I was trying to swap two arrays using pointers. What I wanted was to swap using a call by reference.

This code I wrote is given below

#include <stdio.h>
#include <stdlib.h>

void swap(int **a, int **b) {
    int *temp = (int*)malloc(5*sizeof(int));
    
    temp = *a;
    *a = *b;
    *b = temp;
}

int main() {
    int arr1[] = {1, 2, 3, 4, 5};
    int arr2[] = {6, 7, 8, 9, 10};

    swap((int**)&arr1, (int**)&arr2);

    for(int i=0; i<5; i++) printf("%d\t", arr1[i]);
    printf("\n");
    for(int i=0; i<5; i++) printf("%d\t", arr2[i]);
    printf("\n");
}

The output of the code is:

6   7   3   4   5   
1   2   8   9   10

instead of:

6   7   8   9   10  
1   2   3   4   5

What did I do wrong?

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
  • 3
    Think a little bit about that `malloc` call, and the assignment `temp = *a`... What happens with the memory that is allocated by `malloc`? If there any way to `free` it? – Some programmer dude Oct 31 '22 at 11:55
  • 2
    And `(int**)&arr1` is not correct. The fact that you need to explicitly cast the result of `&arr1` is an indication of that. – Some programmer dude Oct 31 '22 at 11:56
  • 2
    A pointer to an array is not a pointer to a pointer. You misinterpret the first 2 integers as a pointer and swap them. A pointer seems to have same size as 2 integers. That is why the first 2 values are swapped. – Gerhardh Oct 31 '22 at 11:56
  • And anyway, you're not really swapping arrays, you attempt to swap pointer to the arrays (which will not swap the arrays themselves). – Some programmer dude Oct 31 '22 at 11:56

4 Answers4

2

First of all lets fix your swap function:

void swap(int **pparr1, int **pparr2)
{
    int *temp = *pparr1;
    *pparr1 = *pparr2;
    *pparr2 = temp;
}

This function now swaps the two pointers that parr1 and parr2 are pointing to.

Then lest update the way to call this function:

int *parr1 = arr1;  // Make parr1 point to the first element of arr1
int *parr2 = arr2;  // Make parr2 point to the first element of arr2

// Swap the two pointers
swap(&parr1, &parr2);

And then we print using the pointers instead of the arrays:

printf("arr1 = [ ");
for (unsigned i = 0; i < 5; ++i)
{
    printf("%d ", parr1[i]);
}
printf("]\n");

printf("arr2 = [ ");
for (unsigned i = 0; i < 5; ++i)
{
    printf("%d ", parr2[i]);
}
printf("]\n");

This will print the arrays as if they were swapped, but in fact the arrays themselves are not swapped. You can verify by printing using the arrays arr1 and arr2 themselves.

If you want to swap the actual contents of the arrays, you need a loop to actually swap the elements themselves:

// Function to swap two integer values
void swap(int *p1, int *p2)
{
    int temp = *p1;
    *p1 = *p2;
    *p2 = temp;
}

// Function to swap two arrays
void swap_arrays(int *pa1, int *pa2, size_t size)
{
    for (size_t i = 0; i < size; ++i)
    {
        // Swap each separate element of the arrays
        swap(&pa1[i], &pa2[i]);
    }
}

Call like

swap_arrays(arr1, arr2, 5);

After this the contents of the arrays themselves have been swapped.

Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
2

In this call

swap((int**)&arr1, (int**)&arr2);

the passed pointers point to first elements of the arrays arr1 and arr2.

So within the function swap

void swap(int **a, int **b) {
    int *temp = (int*)malloc(5*sizeof(int));
    
    temp = *a;
    *a = *b;
    *b = temp;
}

dereferencing the pointers like for example

*a

results in reading the first two elements of the array arr1 as a pointer because in your system sizeof( int * ) is equal to 2 * sizeof( int ).

That is values of first two elements of the arrays are interpreted as values of pointers.

And the obtained output confirms that

6   7   3   4   5   
1   2   8   9   10
^^^^^

Moreover the function has a memory leak because at first memory was allocated dynamically and its address was assigned to the pointer temp and then the pointer was reassigned

    int *temp = (int*)malloc(5*sizeof(int));
    
    temp = *a;

So the address of the allocated memory was lost and the memory was not freed.

You can not swap arrays such a way. You need to swap each pair of elements of the arrays.

The function can look for example the following way

void swap( int *a, int *b, size_t n ) 
{
    for ( int *p = a; p != a + n; ++p, ++b )
    {
        int temp = *p;
        *p = *b;
        *b = temp;
    }
}

And the function is called like

swap( arr1, arr2, 5 );

A less flexible function can look the following way

void swap( int ( *a )[5], int ( *b )[5] ) 
{
    for ( int *p1 = *a, *p2 = *b; p1 != *a + 5; ++p1, ++p2 )
    {
        int temp = *p1;
        *p1 = *p2;
        *p2 = temp;
    }
}

And the function is called like

swap( &arr1, &arr2 );
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
1

swap((int**)&arr1, (int**)&arr2);

It looks like your idea is to have swap swap the address of arr1 with the address of arr2. Variables in C, including arrays, do not work this way, and this idea cannot possibly work.

arr1 is a name for the array. It is not the address of the array, nor is it a variable whose value is the address of the array. There is no pointer to the array that can be swapped with a pointer to the array arr2.

When the compiler processes int arr1[] = {1, 2, 3, 4, 5};, it creates an array of five int, and then it uses arr1 to refer to that array. Wherever arr1 is used, the compiler creates code that will refer to the array. It does not do this by storing the address of the array in some memory location you can swap with another address. It has ways to access the array using registers and processor instructions set up for that.

In swap((int**)&arr1, (int**)&arr2);, you take the addresses of arr1 and arr2 and convert them to int **. The address of arr1 is the address where its contents are. What is at that address in memory is the int elements, not a pointer. So the int ** type does not make sense for this; the address of arr1 is not the address of an int *.

The reason you get the output you do is that int objects are four bytes in your C implementation and int * objects are eight bytes. So when temp = *a; attempts to get the int * that a should be pointing to, it gets the first eight bytes in arr1. Then *a = *b; replaces those eight bytes with the first eight bytes in arr2. And *b = temp; replaces the first eight bytes in arr2 with the saved eight bytes from arr1.

Note that, in these three statements, temp is used to hold eight bytes from arr1. The value it was given in int *temp = (int*)malloc(5*sizeof(int)); is lost when temp is assigned another value in temp = *a;.

Also note that this behavior you observed in this instance is not specified by the C standard, and different behavior can result in other circumstances, including alignment errors and surprising transformations of the program by compiler optimization.

Eric Postpischil
  • 195,579
  • 13
  • 168
  • 312
1

You cannot assign whole arrays to each other in a single operation using =; you must either use a library function like memcpy or assign each element individually.

Under most circumstances, expressions of type "N-element array of T" will be converted, or "decay", to expressions of type "pointer to T" and the value will be a pointer to the first element. If you called the function as

swap( arr1, arr2 );

it would be equivalent to writing

swap( &arr1[0], &arr2[0] );

So you're already passing the arrays "by reference", just not in the way you're probably thinking.

The easiest way to do what you want would be something like this:

void swap( int *a, int *b, size_t count )
{
  for ( size_t i = 0; i < count; i++ )
  {
    int tmp = a[i];
    a[i] = b[i];
    b[i] = tmp;
  }
}

Alternately, you could use memcpy with a temporary array:

void swap( int *a, int *b, size_t count )
{
  int tmp[count]; // C99 or later
  memcpy( tmp, a, count );
  memcpy( a, b, count );
  memcpy( b, tmp, count );
}

In this case tmp is a variable-length array, whose size is provided by a runtime value. VLAs are useful for things like this if the array size isn't too big, but they're only available for C99 and later, and are optional after C2011. If your arrays are very large or VLAs aren't available, then this would be a case for dynamic memory:

void swap( int *a, int *b, size_t count )
{
  int *tmp = calloc( count, sizeof *tmp );
  if ( tmp )
  {
    memcpy( tmp, a, count );
    memcpy( a, b, count );
    memcpy( b, tmp, count );
    free( tmp );
  }
}

The first method (swapping each element individually) would be my preference. It avoids any kind of dynamic memory management, it's straightforward, it's not making a bunch of library calls, etc.

In all three cases, you'd call it as

swap( arr1, arr2, 5 );

Beware explicit casts, especially pointer casts - they're usually a sign you're doing something wrong.

John Bode
  • 119,563
  • 19
  • 122
  • 198