Swap strings using strcpy() in a function

Question

i know this is a very simple question but i'm new and a bit down here.. i'm learning more about how to use functions, and i can get this to work if it was simply in main, or i can use another way, but i'm trying to solve it using strcpy(), and i dont want to change the main, everythijg should be in the function, any help is very much appreciated.

char swap_char(char *s1, char *s2) {
    printf("before swapping : swap_char(\"%s\", \"%s\")\n", s1, s2);

    char temp[50];
    
    strcpy(temp, s1);
    strcpy(s1, s2);
    strcpy(s2, temp);
    
    printf("After swapping : swap_char(\"%s\", \"%s\")\n", s1, s2);
}


main() {
    swap_char("please","work");
}

Does this answer your question? [Why is this string reversal C code causing a segmentation fault?](https://stackoverflow.com/questions/1614723/why-is-this-string-reversal-c-code-causing-a-segmentation-fault) — kaylum, Nov 30 '21 at 21:30
Swapping strings is a very poor idea unless they are of exactly the same length. Also, don't expect this to work on string literals. — Cheatah, Nov 30 '21 at 21:31
The swapping itself is fine, but the target must be in an area that you're allowed to write to, and string literal are not such areas. — 500 - Internal Server Error, Nov 30 '21 at 21:31
`"please"` is a string literal and cannot be changed. See the duplicate post for more details. Also note that each buffer must be large enough to hold the length of the other string. Try: `char s1[10] = "please"; char s2[10] = "work"; swap_char(s1, s2);` — kaylum, Nov 30 '21 at 21:32
You should structure your code so that you swap only pointers, not the string data itself. — Kaz, Nov 30 '21 at 21:43

score 1 · Answer 1 · answered Nov 30 '21 at 21:37

"please" has 7 characters (including the terminator), but "work" only has 5 characters. There is enough space for the first string to contain "work", but the second string does NOT have enough space to contain "please" in it.
Your code has literal strings "please" and "work", which means they are very likely in READ ONLY memory, and cannot be changed or overwritten.

Fixing these two problems I get:

char swap_char(char *s1, char *s2) {
    printf("before swapping : swap_char(\"%s\", \"%s\")\n", s1, s2);

    char temp[50];
    
    strcpy(temp, s1);
    strcpy(s1, s2);
    strcpy(s2, temp);
    
    printf("After swapping : swap_char(\"%s\", \"%s\")\n", s1, s2);
}


int main() {
    char alpha[20] = "please";  // Declare space 20: MORE than enough space for either string
    char beta[20] = "work";     // Also, use a local array, which is NOT read-only, and can be changed.
    swap_char(alpha,beta);
    return 0;
}

Output

Success #stdin #stdout 0s 5524KB
before swapping : swap_char("please", "work")
After swapping : swap_char("work", "please")

ah, i understood the problem, thanks. what if i put the chars in the function instead of main though? how would that work? — braindamage, Nov 30 '21 at 22:30

Locke · Answer 2 · 2021-11-30T21:47:50.250

The first step is reading the docs to see how it can be used and see if they have any helpful examples.

The first issue here is we don't know if we will have enough space to complete this task. Instead we can malloc space dynamically to make sure we don't run into issues.

// We don't need to return a char
void swap_char(char *s1, char *s2) {
    printf("before swapping : swap_char(\"%s\", \"%s\")\n", s1, s2);

    // Allocate on the heap so we know we will never run out of space on a long input
    char temp = malloc(strlen(s1) + 1);
    
    strcpy(temp, s1);
    strcpy(s1, s2);
    strcpy(s2, temp);

    // Free temporary buffer
    free(temp);
    
    printf("After swapping : swap_char(\"%s\", \"%s\")\n", s1, s2);
}

However, there is a bigger problem here. We don't know if both pointers have enough memory allocated. This is why this approach is not all that practical. Also by passing the string pointers directly instead of using a pointer to a buffer on the stack or heap risks attempting to mutate read-only memory. String constants are loaded along with the assembly instructions in each function into read-only memory on most modern systems. This is good since it prevents a malicious actor or undefined behavior from modifying the function assembly.

char *a = "please";
char *b = "work";

// Create new buffers with the required space to use instead
unsigned int buffer_len = imax(strlen(a), strlen(b)) + 1;
char *a_buffer = malloc(buffer_len);
char *b_buffer = malloc(buffer_len);

// Copy the input strings into our new buffers
strcpy(a_buffer, a);
strcpy(b_buffer, b);

// Finally swap the buffers
swap_char(a_buffer, b_buffer);

As you can see, it isn't very practical but it is possible. A more practical approach is to just swap the pointers held by variables.

void swap_strings(char **s1, char **s2) {
    char *temp = *s1;
    *s1 = *s2;
    *s2 = temp;
}


char *a = "please";
char *b = "work";

printf("Before swapping : swap_char(\"%s\", \"%s\")\n", a, b);
swap_strings(&a, &b);
printf("After swapping : swap_char(\"%s\", \"%s\")\n", a, b);

Swap strings using strcpy() in a function

2 Answers2

Output