Segmentation Fault function that changes the values of 2 strings

Question

The following function changes (or at least should) the content of two strings

void trocar(char *a, char *b){
    char *novo = (char *) malloc(sizeof(char) * 10);

    strcpy(novo, a);
    strcpy(a, b);
    strcpy(b, novo);
}

But when calling the function in main, Segmentation Fault occurs.

int main(int argc, char const *argv[]) {

    char *a = "oi";
    char *b = "tchau";
    trocar(a, b);
    printf("a: %s, b: %s\n", a, b);

    return 0;
}

I could not understand what is causing this error.

What causes this? And if possible, how would a function change the value of two strings correctly?

Answer 1

You didn’t copy to the area of your code, so you tried to swap values from the static area of the code and this is a read-only area. The data in your code is neither in automatic memory nor in dynamic memory which is what you want to move. This is how it works:

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

void trocar(char *a, char *b) {
    char *novo = malloc(10);
    strcpy(novo, a);
    strcpy(a, b);
    strcpy(b, novo);
}

int main() {
    char *a = malloc(10);
    strcpy(a, "oi");
    char *b = malloc(10);
    strcpy(b, "tchau");
    trocar(a, b);
    printf("a: %s, b: %s\n", a, b);
}

Behold working in the ideone. And in the repl it.. Also put on the Github for future reference.

But the way you trade in truth because it’s more efficient would be using only pointers:

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

void trocar(char **a, char **b) {
    char *novo = *a;
    *a = *b;
    *b = novo;
}

int main() {
    char *a = malloc(3);
    strcpy(a, "oi");
    char *b = malloc(6);
    strcpy(b, "tchau");
    trocar(&a, &b);
    printf("a: %s, b: %s\n", a, b);
}

Behold working in the ideone. And in the repl it.. Also put on the Github for future reference.

But if you really want to change the value at least you should use automatic memory that is more secure and efficient, dynamic memory should be used as a last resort, and it doesn’t seem necessary in this case (it would only be if you have something that I don’t know just by seeing this code). Then the data is already putting where it should. But still copies what is still very inefficient.

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

void trocar(char *a, char *b) {
    char novo[10];
    strcpy(novo, a);
    strcpy(a, b);
    strcpy(b, novo);
}

int main() {
    char a[10] = "oi";
    char b[10] = "tchau";
    trocar(a, b);
    printf("a: %s, b: %s\n", a, b);
}

Behold working in the ideone. And in the repl it.. Also put on the Github for future reference.

Note that in your example I had to change the memory allocation because with the exchange you have to make sure that everyone involved has room for the larger one. For a general example this is much more complicated, for this example the largest is 6, we are seeing in the code, but you used 10 in the temporary, I imagine you know what you are doing, that could change the code to be more than 6, but not more than 10 (9 usable characters). So I fixed it. And then in the example of the stack also had to use 10. Yes, there is waste in both, which in this case changes little. So the second solution is also the best in this aspect of memory consumption, is even more conceptually correct, is not only efficiency.