Why does this program give segmentation fault?

Question

It's a beginners question: Why is this breaking/giving an error?

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

char *strtrim_right(char *p)
{
  char *end;
  int len;
  len = strlen( p);
  while (*p && len)
    {
    end = p + len-1;
    if(isalpha(*end))
     *end =0;
   else 
    break;      
    }
  return(p);
}


int main ()  
{  
  char *x="PM123BFD";
  strtrim_right(x);
  printf("%s", x);
  return 0;
}

What error? What line? What compiler? What platform? How do you expect we help you if you don't provide information? — Alexandre C., Sep 17 '10 at 12:00
"deprecated conversion from string constant to ‘char*’" for x; rather use a char array: char x[] = ... — slashmais, Sep 17 '10 at 12:19
The standard answer: because you invoked undefined behavior. :-) — R.. GitHub STOP HELPING ICE, Sep 17 '10 at 17:04

score 11 · Accepted Answer · answered Sep 17 '10 at 12:04

11

Change

char *x="PM123BFD";

to

char x[]="PM123BFD";

You cannot modify a string literal, so instead pass the function a char array which it can modify.

answered Sep 17 '10 at 12:04

codaddict

445,704
82
492
529

score 5 · Answer 2 · answered Sep 17 '10 at 12:01

I don’t see why it should break – I would rather expect an infinite loop: the while condition will always be true and the loop will never be left.

Rework the loop condition, it’s borked. Then look at the variables you have: you never change the values of either p or len. This isn’t right. Furthermore, the code inside the loop is much more complicated than need be. Think about whether you really need three variables here.

score 0 · Answer 3 · edited Sep 17 '10 at 15:22

0

Ok thanks to the 2 answers above here is what seems to be ok now:

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

char *strtrim_right(char *p)
{
  char *end;
  int len;
  len = strlen( p);
  while (*p && len)
    {
    end = p + len-1;
    if(isalpha(*end))
     *end =0;
   else 
    break;
len = strlen(p);
    }
  return(p);
}


int main ()  
{  
  char x[]="PM123BFD";
  strtrim_right(x);
  printf("%s", x);
  return 0;
}

edited Sep 17 '10 at 15:22

codaddict

445,704
82
492
529

answered Sep 17 '10 at 12:09

francogrex

465
1
4
17

1

why is strtrim_right returning a char pointer if you aren't using it? – jmtd Sep 17 '10 at 12:18
1

there’s still a lot wrong with this code. The `while` condition and the code inside the loop are still hopelessly complicated. – Konrad Rudolph Sep 17 '10 at 12:21
void strtrim_right(char *p) { p += strlen(p) - 1; while(isalpha(*p)) { *p-- = 0; } } – jmtd Sep 20 '10 at 12:29

Why does this program give segmentation fault?

3 Answers3

Linked

Related