12

Why does the following code return with a segmentation fault? When I comment out line 7, the seg fault disappears.

int main(void){
      char *s;
      int ln;
      puts("Enter String");
      // scanf("%s", s);
      gets(s);
      ln = strlen(s); // remove this line to end seg fault
      char *dyn_s = (char*) malloc (strlen(s)+1); //strlen(s) is used here as well but doesn't change outcome
      dyn_s = s;
      dyn_s[strlen(s)] = '\0';
      puts(dyn_s);
      return 0;
    }

Cheers!

bomp
  • 309
  • 1
  • 4
  • 12
  • @Lundin, Please point out what this is a duplicate of? If you are referring to https://stackoverflow.com/questions/37549594/why-do-i-get-a-mysterious-crash-or-segmentation-fault-when-i-copy-scan-data-to, I'd state that my question is 4 years older – bomp Jul 28 '16 at 11:53

5 Answers5

20

s is an uninitialized pointer; you are writing to a random location in memory. This will invoke undefined behaviour.

You need to allocate some memory for s. Also, never use gets; there is no way to prevent it overflowing the memory you allocate. Use fgets instead.

Community
  • 1
  • 1
Oliver Charlesworth
  • 267,707
  • 33
  • 569
  • 680
  • Thanks a lot Oli! Pardon me for being a pest, but, am I to understand that the addition of the line I mentioned, causes the undefined behavior to result in a seg fault. Could you help me visualize the causality a bit more? – bomp May 19 '12 at 20:19
  • @resonant_fractal: It's difficult to say. Undefined behaviour, by definition, means that your program could do anything, including failing in unpredictable ways. I could make guesses, but the only way to know for sure would be to look at the machine code that your compiler produced. – Oliver Charlesworth May 19 '12 at 20:21
  • because the `strlen()` function try to access a space that don't exists. same as `strlen(NULL)`. Inside function there something like: `size_t strlen(char *s) { char *p=s; while(` ***p++** `)` < it causes the segmentation fault. – The Mask May 19 '12 at 20:38
  • @TheMask Thanks! But what about the malloc in line 8 using the same strlen expression? The whole program runs without any errors if i just remove line 7. Or are you guys getting something different? – bomp May 19 '12 at 21:17
  • If I remove the line 7 or 8 I will get same error. because actually, the error starts on `gets()` call and not on `strlen()` as I said before(but the reason is same),sorry. If you don't know about `gdb`,see about,I recommend use it. It can be a lot useful for detecting errors. For example, if you run program using gdb a.out, you will get: `Enter String foo Program received signal SIGSEGV, Segmentation fault. _IO_gets (buf=0x282ff4 "|M\025") at iogets.c:55`. – The Mask May 19 '12 at 22:01
  • I'm assuming that you're programming on a POSIX environment. as @Oli has mentioned, use `fgets()` instead of `gets()`(it will removed from C std on future), or use this information on `scanf()` function: `%301s` it will read until 300 characters from `stdin`. the rest,1, is reserved for null byte terminator string,`\0`. – The Mask May 19 '12 at 22:02
4

Catastrophically bad:

int main(void){
      char *s;
      int ln;
      puts("Enter String");
      // scanf("%s", s);
      gets(s);
      ln = strlen(s); // remove this line to end seg fault
      char *dyn_s = (char*) malloc (strlen(s)+1); //strlen(s) is used here as well but doesn't change outcome
      dyn_s = s;
      dyn_s[strlen(s)] = '\0';
      puts(dyn_s);
      return 0;
    }

Better:

#include <stdio.h>
#define BUF_SIZE 80

int 
main(int argc, char *argv[])
{
      char s[BUF_SIZE];
      int ln;
      puts("Enter String");
      // scanf("%s", s);
      gets(s);
      ln = strlen(s); // remove this line to end seg fault
      char *dyn_s = (char*) malloc (strlen(s)+1); //strlen(s) is used here as well but doesn't change outcome
      dyn_s = s;
      dyn_s[strlen(s)] = '\0';
      puts(dyn_s);
      return 0;
    }

Best:

#include <stdio.h>
#define BUF_SIZE 80

int 
main(int argc, char *argv[])
{
      char s[BUF_SIZE];
      int ln;
      puts("Enter String");
      fgets(s, BUF_SIZE, stdin); // Use fgets (our "cin"): NEVER "gets()"

      int ln = strlen(s); 
      char *dyn_s = (char*) malloc (ln+1);
      strcpy (dyn_s, s);
      puts(dyn_s);
      return 0;
    }
paulsm4
  • 114,292
  • 17
  • 138
  • 190
  • Better still: `char *dyn_s = malloc(ln + 1);`. And note that `fgets()` leaves the terminating `'\n'` in the buffer; `gets()` discards it (but `gets()` is still catastrophically bad). – Keith Thompson May 19 '12 at 20:19
  • Cool! Thanks. Any great resources you have links to, wrt the behavior of commonly used IO functions? – bomp May 19 '12 at 20:26
  • And besides `cast` `malloc()` result removal,check if you have no received a `NULL` value. `char *dyn_s = malloc(ln + 1); if(!dyn_s) { printf("No mem!\n"); exit(EXIT_FAILURE);}` or can you get another segmentation fault. – The Mask May 19 '12 at 22:10
  • @resonant_fractal: check out: http://www.cplusplus.com/reference/clibrary/cstdio/ – The Mask May 19 '12 at 22:12
1

Your scanf("%s", s); is commented out. That means s is uninitialized, so when this line ln = strlen(s); executes, you get a seg fault.

It always helps to initialize a pointer to NULL, and then test for null before using the pointer.

octopusgrabbus
  • 10,555
  • 15
  • 68
  • 131
  • Thanks! Will do the NULL pointer initialization and testing more often. I don't think I get the scanf part. Shouldn't the scanf fail too? After all, the pointer is not initialized to anything and may point to some arbitrary and illegal location as the start position of the string, hence a seg fault? I commented it out since including it was also resulting in seg faults. Or have I failed to understand something? – bomp May 21 '12 at 18:45
  • This answer is **wrong**. If you initialize that pointer to NULL, that will also result in UB - NULL is never to be dereferenced, not even by scanf... –  Sep 15 '12 at 10:12
  • I test my pointers to see if they are set to null. If initialized to null before using, then there is a reasonable chance the pointer is not already assigned to valid memory. – octopusgrabbus Sep 15 '12 at 13:53
1

Even better

#include <stdio.h>
int
main(void)
{
  char *line = NULL;
  size_t count;
  char *dup_line;

  getline(&line,&count, stdin);
  dup_line=strdup(line);

  puts(dup_line);

  free(dup_line);
  free(line);

  return 0;
}
KAction
  • 1,977
  • 15
  • 31
  • Hi! I have never used the getline() and strdup() functions. I don't think i have seen them before either. Thanks for introducing them to me ;). Umm unfortunately your code runs into a seg fault! – bomp Oct 18 '12 at 09:58
  • Strange. Warning about implicit declaration, correct work for me. GNU/Linux system, gcc 4.6.3. – KAction Oct 18 '12 at 10:10
  • `line` should be initialized with NULL. Yeah, C can be cruel. – KAction Oct 18 '12 at 10:21
  • Cool! Thanks KAction! Been a little busy lately. I promise I'll look deeper into the inner machinations of your code sometime soon. Have a great weekend. Cheers! – bomp Oct 19 '12 at 12:19
1
char *s  does not have some memory allocated . You need to allocate it manually in your case . You can do it as follows
s = (char *)malloc(100) ;

This would not lead to segmentation fault error as you will not be refering to an unknown location anymore

Subbu
  • 2,063
  • 4
  • 29
  • 42