This function with pointers seemingly won't execute at all-CodePudding

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);
}

CodePudding user response：

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).

CodePudding user response：

The compiler should issue an error or at least a warning message for this line

void swap(p, q);

because it is incorrect.

It is a function declaration with an identifier list but in a function declaration that is not at the same time its definition the identifier list shall be empty.

Instead you need to call the function swap

swap( p, q );

In fact the pointers p and q are redundant. You could also call the function like

swap( &a, &b );