My function that reverses a string gives me "stack smashing detected"

Question

I am trying to write a function that reverses a given string but it gives me "stack smashing detected".

Here is my code:

void reverse(char *str2) {
    int i, j;
    char temp;
    for (i = 0, j = str2[strlen(str2) - 1]; i < j; i++, j--) {
         temp = str2[i];
         str2[i] = str2[j];
         str2[j] = temp;
    }
    str2[strlen(str2)] = '\0';
    printf("%s", str2);
}

Look, take out those expressions like 'j = str2[strlen(str2) - 1', load j etc. before the loop and printf them out. Do not attempt to write 'clever' code - write it so you can debug it. — Martin James, May 22 '22 at 09:17

score 0 · Answer 1 · answered May 22 '22 at 09:14

There are multiple problems in your code:

j should be initialized as j = strlen(str2) - 1, not j = str2[strlen(str2) - 1] which is the value of the last character in the string (if any).
str2[strlen(str2)] = '\0'; is absolutely useless and redundant.

Here is a modified version:

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

void reverse(char *str) {
    for (size_t i = 0, j = strlen(str); i < j--; i++) {
         char temp = str[i];
         str[i] = str[j];
         str[j] = temp;
    }
    printf("%s\n", str);
}

Vlad from Moscow · Answer 2 · 2022-05-22T11:38:14.630

Such a function should return a pointer to the reversed string. That is it is better to declare the function like

char * reverse( char *str2 );

Also the name of the parameter looks strange. So declare the function like

char * reverse( char *s );

There is a typo in the function. The variable j must be initialized like

j = strlen( str2 ) - 1;

instead of

j = str2[strlen(str2) - 1];

This statement

str2[strlen(str2)] = '\0';

is redundant and should be removed.

The function should not output the reversed string. It is the caller of the function that will decide whether to output the reversed string.

Also instead of the type int for indices you should use the type size_t - the return type of the function strlen.

Using your approach the function should look the following way

char * reverse( char *s ) 
{
    if ( *s )
    {
        for ( size_t i = 0, j = strlen( s ) - 1; i < j; i++, j-- ) 
        {
            char temp = s[i];
            s[i] = s[j];
            s[j] = temp;
        }
    }

    return s;
}

Here is a demonstration program.

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

char * reverse( char *s ) 
{
    if ( *s )
    {
        for ( size_t i = 0, j = strlen( s ) - 1; i < j; i++, j-- ) 
        {
            char temp = s[i];
            s[i] = s[j];
            s[j] = temp;
        }
    }

    return s;
}

int main(void)
{
    char s[] = "Hello World!";
    puts( s );
    puts( reverse( s ) );
}

Its output is

Hello World!
!dlroW olleH

My function that reverses a string gives me "stack smashing detected"

2 Answers2