Search code examples
cfunction-callfunction-declaration

This function with pointers seemingly won't execute at all


I'm trying to make a program that swaps 2 vars in a function with bitwise operators, but the function won't do anything. I'm pretty sure I got the basic logic of the swapping with XOR correct (I checked by doing some operations by hand, and checked in main), but for some reason the function block just won't do anything. Also I am very new to C programming, so forgive me for not spotting some obvious silly mistake.

The code is:

#include <stdio.h>


void swap(int *a, int *b)
{
    printf("value of a=%d b=%d before swapping\n", *a, *b);
    *a = *a ^ *b;
    *b = *a ^ *b;
    *a = *a ^ *b;
    printf("value of a=%d b=%d after swapping\n", *a, *b);
}


int main()
{
    int a, b, *p, *q;
    printf("Enter 2 numbers to be swapped: \n");
    scanf("%d %d", &a, &b);
    p = &a;
    q = &b;
    void swap(p, q);
    printf("\nfinal %d %d", a, b);
}

Solution

  • gcc even without -Wall points out the exact location of the bug. Just compile the code and see where it points:

    warning: parameter names (without types) in function declaration
          |     void swap(p, q);
          |     ^~~~
    

    The ^ arrow saying "bug here". Simply drop the void in the function call.

    In addition, XOR swaps is one of those "obfuscate just for the heck of it" things. The only purpose for it seems to be posing, since temp variable swaps often result in more efficient code. See this benchmarking on x86 gcc -O3:

    void xor_swap(int *a, int *b)
    {
        *a = *a ^ *b;
        *b = *a ^ *b;
        *a = *a ^ *b;
    }
    
    void tmp_swap(int *a, int *b)
    {
      int tmp = *a;
      *a = *b;
      *b = tmp;
    }
    
    xor_swap:
            mov     eax, DWORD PTR [rdi]
            xor     eax, DWORD PTR [rsi]
            mov     DWORD PTR [rdi], eax
            xor     eax, DWORD PTR [rsi]
            mov     DWORD PTR [rsi], eax
            xor     DWORD PTR [rdi], eax
            ret
    
    tmp_swap:
            mov     eax, DWORD PTR [rdi]
            mov     edx, DWORD PTR [rsi]
            mov     DWORD PTR [rdi], edx
            mov     DWORD PTR [rsi], eax
            ret
    

    The tmp_swap is both more readable and more efficient.


    EDIT: Actually I just now played a round a bit more with this and the inefficiency of the XOR version mostly comes from assumptions that the pointers might alias. Updating the function to void xor_swap(int* restrict a, int* restrict b) made it equally fast as the tmp_swap (but still less readable).