Reversing a string in C iteratively

Question

I am trying to reverse a string in C, I was able to accomplish this but not without a side effect that I don't quite understand. Here is my code to reverse a string:

void reverseString(char *toReverse, char *reverse) {
    int i = strlen(toReverse);
    int counter = 0;
    for (i; toReverse[counter] != '\0'; i--) {
        reverse[i] = toReverse[counter];
        counter++;
    }
}

int main(int argc, char *argv[]) {
    char reverse[strlen(argv[1]) + 1];
    reverse[strlen(reverse)] = '\0';
    reverseString(argv[1], reverse);
    printf("The reverse string is '%s'", reverse);
}

When give a string this correctly reverse the string but also adds some additional data, for example:

Given the string abc123 the string 321cbaub◄¥u"ñç« is returned

Why is this happening and how can I fix it?

Insread of:'reverse[strlen(reverse)] = '\0';' You need 'reverse[strlen(argv[1])] = '\0';' — zr., Feb 20 '16 at 17:48

score 2 · Answer 1 · answered Feb 20 '16 at 17:48

You should give a null termination \0 in your reverse string to use printf with %s format to print the string you reversed correctly.

void reverseString(char* toReverse, char* reverse) {
    int i = strlen(toReverse);
    int counter = 0;
    for (i; toReverse[counter] != '\0'; i--) {
        reverse[i] = toReverse[counter];
        counter++;
    }
    reverse[counter] = '\0'; //add this
}

Also, sometimes for dynamically allocated stuff, it would be best practice to use malloc:

reverse = (char*)malloc(i + 1);

Becomes something like this:

void reverseString(char* toReverse, char* reverse) {
    int i = strlen(toReverse);
    int counter = 0;
    reverse = (char*)malloc(i + 1);
    for (i; toReverse[counter] != '\0'; i--) {
        reverse[i] = toReverse[counter];
        counter++;
    }
    reverse[counter] = '\0'; //add this
}

Why use memory allocation in `reverseString`? `i` must be decremented before the first loop iteration. — chqrlie, Feb 20 '16 at 17:59

score 1 · Answer 2 · answered Feb 20 '16 at 17:49

I will say reverse[counter] = '\0'; add this in the function it self. That will look like

void reverseString(char* toReverse, char* reverse) {
    int i = strlen(toReverse);
    int counter = 0;
    for (i; toReverse[counter] != '\0'; i--) 
    {
        reverse[i] = toReverse[counter];
        counter++;
    }
    reverse[counter] = '\0'; //add this
}

This works!

chqrlie · Accepted Answer · 2016-02-20T18:25:13.870

Your code has several problems:

The way you set the null terminator for reversed in main is incorrect because you use strlen(reverse) when reverse is still uninitialized. That's the main reason you get garbage output, because the null terminator is not set at the correct offset, but this is undefined behavior, passing an uninitialized array to strlen could have worse consequences.
The null terminator should be set in reverseString for consistency and simplicity.
You should decrement i before copying the character, otherwise you create the reversed string shifted one position to the right.
You should end the printf format string with a linefeed
main should return 0.

Here is a corrected version:

void reverseString(const char *toReverse, char *reverse) {
    int i = strlen(toReverse);
    int counter = 0;
    reverse[i] = '\0';  /* set the null terminator */
    while (toReverse[counter] != '\0') {
        reverse[--i] = toReverse[counter++];
    }
}

int main(int argc, char *argv[]) {
    char reverse[strlen(argv[1]) + 1];
    reverseString(argv[1], reverse);
    printf("The reverse string is '%s'\n", reverse);
    return 0;
}

score 0 · Answer 4 · answered Feb 20 '16 at 17:49

0

Your string isn't null-terminated, that's why you're getting rubbish after the reversed string.

So, you should first do reverse[i+1]=0; in your reverseString.

answered Feb 20 '16 at 17:49

ForceBru

43,482
10
63
98

Reversing a string in C iteratively

4 Answers4