Getting "Segmentation fault Core Dumped Error " while reversing a string

Question

I am trying to learn C, I am getting this error while reversing a string. I am not well versed with memory allocation stuffs, can you please point out where I am doing wrong.

#include<stdio.h>
#include<string.h>
char* strrev(char *str);
int main()
{
  char str[1000];
  char *str1;
  printf("Please enter a String\n");
  gets(str);

  str1=strrev(str);
  puts(str1);

}

char* strrev(char *str)
{
  char *str1;
  int i,length,c;
  length=strlen(str);

  for (i=length-1;i<=0;i--)
  {
     *(str1+c) = *(str+i);
     c++;
  }
  *(str1+c) ='\0';
  return str1;

}

Using gets is discouraged because this function cannot prevent buffer overrun. — MikeCAT
– MikeCAT, Commented Nov 4, 2015 at 12:46
Your loop in strrev will never run for strings with a length of more than one characters. For zero-length string you will have undefined behavior. — Some programmer dude
– Some programmer dude, Commented Nov 4, 2015 at 12:48
Unrelated to your errors, maybe just check all the other posts already on SO which does exactly the same thing, such as this one? — Lundin
– Lundin, Commented Nov 4, 2015 at 12:48
Since you are just starting to learn c, now would be a good time to learn how to use GDB. It's a debugger that allows you to step through your code. It helps you learn the ins and outs of your code. It's simple and makes debugging errors like this a breeze. Here's a simple starter: cs.umd.edu/~srhuang/teaching/cmsc212/gdb-tutorial-handout.pdf — fpes
– fpes, Commented Nov 4, 2015 at 12:57
You're also not initializing c, which could be anything. So you're writing to a random location in memory. — Tom Karzes
– Tom Karzes, Commented Nov 4, 2015 at 13:11

gmoniava · Accepted Answer · 2015-11-04 12:54:43Z

2

  for (i=length-1;i<=0;i--)

This will never (unless string is 0 or 1 character long) run due to i<=0, should be i>=0 probably.

Also in general you need to make pointer point to some valid memory in order to be able to dereference it. In your case you should probably use malloc for allocating sufficient number of bytes, and assign its result to str1. Then you can write to it as you are doing.

edited Nov 4, 2015 at 12:54

answered Nov 4, 2015 at 12:47

gmoniava

28.9k9 gold badges61 silver badges101 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

MikeCAT Over a year ago

Strictly speaking, not never. What if the length of string str is zero or one ?

gmoniava Over a year ago

@MikeCAT: Yes, incorporated

Community · Accepted Answer · 2017-05-23 10:27:17Z

2

Inside your strrev() function, str1 is not allocated memory. Hence, *(str1+c) = *(str+i); is UB.

Then, c is an automatic local variable which is not initialized before use. Also UB.

Next, as Giorgi mentioned, correct your for loop.

That said, don't use gets(), it suffers from buffer overrun issues. Use fgets() instead.

edited May 23, 2017 at 10:27

CommunityBot

11 silver badge

answered Nov 4, 2015 at 12:46

Sourav Ghosh

135k17 gold badges192 silver badges271 bronze badges

Comments

Vlad from Moscow · Accepted Answer · 2015-11-04 13:07:08Z

0

What you are trying to do is not reversing a string. Neither string is reversed in your program. You are trying to copy one string in another string in the reverse order.

So the sring where you are going to copy the original string in the reverse order shall have enough space to store the copied characters.

However in your function

char* strrev(char *str)
{
  char *str1
  //...

pointer str1 was not initialized and does not point to an extent of memory of the appropriate size.

So your program has undefined behaviour.

Take also in account that function gets is unsafe and is not supported by the C standard any more. Use function fgets instead.

If your compiler supports Variable Length Arrays (VLA) then the program can look the following way

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

char * reverse_copy( char *s2, const char *s1 )
{
    size_t n = strlen( s1 );
    size_t i = 0;

    for ( ; i < n; i++ ) s2[i] = s1[n-i-1];

    s2[i] = '\0';

    return s2;
}

int main( void )
{
    char s1[1000];

    printf( "Please enter a String: " );
    fgets( s1, sizeof( s1 ), stdin );

    size_t n = strlen( s1 );

    if ( n != 0 && s1[n-1] == '\n' ) s1[n-1] = '\0';

    char s2[n + 1];

    puts( s1 );
    puts( reverse_copy( s2, s1 ) );

    return 0;
}

If to enter for example

Hello Asfakul Islam

then the output will look like

Please enter a String: Hello Asfakul Islam
Hello Asfakul Islam
malsI lukafsA olleH

Otherwise if your compiler does not support VLA(s) you need to dynamically allocate an array of the appropriate size. In this case the program can look for example the following way

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

char * reverse_copy( char *s2, const char *s1 )
{
    size_t n = strlen( s1 );
    size_t i = 0;

    for ( ; i < n; i++ ) s2[i] = s1[n-i-1];

    s2[i] = '\0';

    return s2;
}

int main( void )
{
    char s1[1000];

    printf( "Please enter a String: " );
    fgets( s1, sizeof( s1 ), stdin );

    size_t n = strlen( s1 );

    if ( n != 0 && s1[n-1] == '\n' ) s1[n-1] = '\0';

    char *s2 = malloc( ( n + 1 ) * sizeof( char ) );

    puts( s1 );
    puts( reverse_copy( s2, s1 ) );

    free( s2 );

    return 0;
}

answered Nov 4, 2015 at 13:07

Vlad from Moscow

313k27 gold badges204 silver badges358 bronze badges

2 Comments

redsoxlost Over a year ago

What do you mean by not initialized? What do I need to to initialize a pointer? Should I just assign a value?

Vlad from Moscow Over a year ago

@AsfakulIslam You are using a pointer that has undetermined value. You need 1) to allocate memory where you are going to write the reversed string and 2) to assign the pointer by the address of the allocated memory.

MikeCAT · Accepted Answer · 2015-11-04 12:52:12Z

0

You don't initialize str1 in strrev()
If you enter string whose length is 2 characters or more, i<=0 is false and the block inside the for loop won't be executed.
*(str1+c) ='\0'; will cause the crash because the value of str1 is indeterminate and you have few chance that str1 points some valid place.

UPD: c in strrev() is also uninitialized, and it will cause some trouble.

answered Nov 4, 2015 at 12:52

MikeCAT

75.1k11 gold badges50 silver badges71 bronze badges

Comments

Haris · Accepted Answer · 2015-11-04 12:55:29Z

0

In your function, you did not initialize c

int i,length,c;

and are using it inside the for loop

*(str1+c) = *(str+i);

Plus other problems are there..

1) str1 inside the function is not allocated memory.

2) This loop will never get executed, as the condition in for (i=length-1;i<=0;i--) is never true (unless string is 0 or 1 character long).

3) Do not use gets(), it is deprecated. Instead use fgets()

edited Nov 4, 2015 at 12:55

answered Nov 4, 2015 at 12:49

Haris

12.3k6 gold badges48 silver badges75 bronze badges

1 Comment

MikeCAT Over a year ago

i<=0 do become true if length<=1.

Collectives™ on Stack Overflow

Getting "Segmentation fault Core Dumped Error " while reversing a string

5 Answers 5

2 Comments

Comments

2 Comments

Comments

1 Comment

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

2 Comments

Comments

2 Comments

Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Linked

Related