Segmentation fault occuring when trying reverse a string

Question

So I have this function which is supposed to reverse a string and then return it

char *my_revstr(char *str)
{
    int i = 0;
    int j = 0;
    int k = 0;

    while(str[j] != '\0') {
        j++;
    }
    j--;
    while(i < j) {
        k = str[i];
        str[i] = str[j];
        str[j] = k;
        i++;
        j--;
    }
    return (str);
}

But whenever I try to run it I have segmentation fault, and I'll be honest I don't very know why. I hope someone can help me fix this ^^.

the code seems fine to me, have you tried debugging it? also does it consistently crash with every test case? — Gilad, Oct 06 '20 at 08:22
Please show a [mcve]. The way you call `my_revstr` may be the problem. The `my_revstr` function itself looks correct to me. — Jabberwocky, Oct 06 '20 at 08:27
There's a significant chance this is a duplicate of [**Why do I get a segmentation fault when writing to a `char *s` initialized with a string literal, but not `char s[]`?**](https://stackoverflow.com/questions/164194/why-do-i-get-a-segmentation-fault-when-writing-to-a-char-s-initialized-with-a?rq=1) — Andrew Henle, Oct 06 '20 at 08:56

Vlad from Moscow · Accepted Answer · 2020-10-06T10:08:32.610

I am sure that you have a segmentation fault due to passing to the function a string literal something like

my_revstr( "Hello" );

You may not change a string literal. Any attempt to change a string literal results in undefined behavior.

You should write like

char s[] = "Hello";
my_revstr( s );

Pay attention to that the variables i and j should have the type size_t because the type int can be not enough large to store sizes of strings.

The function can be defined for example the following way as it is shown in the demonstrative program below,

#include <stdio.h>

char * my_revstr( char *s )
{
    size_t n = 0;

    while ( s[n] != '\0' ) ++n;

    if ( n != 0 )
    {
        for ( size_t i = 0; i < --n; ++i )
        {
            char c = s[n];
            s[n] = s[i];
            s[i] = c;
        }
    }

    return s;
}

int main(void) 
{
    char s[] = "Hello";
    
    puts( s );
    puts( my_revstr( s ) );

    return 0;
}

The program output is

Hello
olleH

The function can be defined also for example the following way

char * my_revstr( char *s )
{
    size_t n = 0;

    while ( s[n] != '\0' ) ++n;

    for ( size_t i = 0; i < n / 2; ++i )
    {
        char c = s[i];
        s[i] = s[n - i - 1];
        s[n - i - 1] = c;
    }

    return s;
}

Or you could implement the function using pointers. For example

char * my_revstr( char *s )
{
    char *first = s, *last = s;

    while ( *last ) ++last;

    if ( first != last )
    {
        for ( ; first < --last; ++first )
        {
            char c = *first;
            *first = *last;
            *last = c;
        }
    }

    return s;
}

If we want to be unnecessarily pedantic about it technically one should say that you "ought" not modify a string literal. Things that cause undefined behaviour aren't forbidden, they just make the program meaningless. — Roflcopter4, Oct 06 '20 at 08:40

Segmentation fault occuring when trying reverse a string

1 Answers1