strcpy(char*,char a[]) is giving incorrect output. (Reversing a string word by word)

Question

What I'm trying to achieve -

Input: (String literal assumed.) This is a string

Output: string a is This

My naive solution:

1. Copy the string literal to an char array.
   Current contents of the char array str[sizeofstring] : This is a string

2. Reverse the array word by word and store it in another array.
   char reverse[sizeofstring]: sihT si a gnirts

3. Traverse array reverse from the last to the 0th position. Store it in char array solution.
   char solution[sizeofstring]: string a is This

4. strcpy(pointertoachar, solution). - Because the function needs to return a pointer to char.

Code:

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

char *reverse(char *input) {
    int n = strlen(input);
    char str[n];

    char reverse[n];
    char solution[n];

    char *solutionp = malloc(sizeof(char) * n);
    strcpy(str, input);

    int last = 0;
    int i = 0;
    int q = 0;

    while (str[i] != '\0') {
        if (str[i] == ' ') {
            printf("i : %d\n", i);
            printf("LAST:%d\n", last);
            for (int t = (i - 1); t >= last; t--) { 
                reverse[q] = str[t];
                q++;
            }
            last = i + 1;
            reverse[q] = ' ';
            q++;
        }
        i++;
    }

    // for the last word.
    for (int cc = i - 1; cc >= last; cc--) {
        reverse[q] = str[cc];
        q++;
    }

    // Traversing from the last index to the first.
    int ii;
    int bb = 0;
    for (ii = n - 1; ii >= 0; ii--) {
        solution[bb] = reverse[ii];
        bb++;
    }

    // This prints the right output.
    // printf("%s\n",solution);

    // Copying from a char array to pointer pointing to a char array.
    strcpy(solutionp, solution);

    return solutionp;
}

int main() {
    char *str = "This is a string";
    char *answer;

    answer = reverse(str);
    printf("%s\n", answer);

    printf("\n");

    return 0;
}

The problem:

Steps 1 to 3 are working as intended. For debugging purpose, I tried printing the output of the array which contains the solution and it worked, but when I copy it to char array pointed by a pointer using strcpy and return the pointer, it prints garbage values along with partially right output.

OUTPUT:
string a is This??Z??

There seems to be some problem in step 4. What am I doing wrong?

Answer 1

The major problem in your code is you allocate your temporary buffers one byte too short. You must make enough room for the final '\0' byte at the end of the strings.

You can simplify the code by using an auxiliary function to copy a block in reverse order:

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

char *reverse_block(char *dest, const char *src, int len) {
    for (int i = 0; i < len; i++) {
        dest[i] = src[len - i - 1];
    }
    dest[len] = '\0';
    return dest;
}

char *reverse_words(const char *string) {
    int i, last;
    int len = strlen(string);
    char temp[len + 1];

    for (i = last = 0; i < len; i++) {
        if (string[i] == ' ') {
            // copy the word in reverse
            reverse_block(temp + last, string + last, i - last);
            temp[i] = ' ';
            last = i + 1;
        }
    }

    // copy the last word in reverse
    reverse_block(temp + last, string + last, len - last);

    // allocate an array, reverse the temp array into it and return it.
    return reverse_block(malloc(len + 1), temp, len);
}

int main(void) {
    const char *string = "This is a string";

    printf("%s\n", string);

    char *solution = reverse_words(string);
    printf("%s\n", solution);

    free(solution);

    return 0;
}

Now you can improve the code further by implementing a function that reverses a block in place. With this, you no longer need a temporary buffer, you can work on the string copy directly and it simplifies the code.