Segmentation fault when using strlen on user input string

Question

I'm trying to understand what's wrong with my code. I have a string composed by words inserted by user input. I've terminated the string so it should be ok.

Then I use another cycle to reverse the direction of the words. But when I call STRLEN on the last word of the string, it gives me segmentation fault. (the reversal part is not done yet, since i'm stuck with this problem).

Why?

Here is the code:

char *frase, c;
int i=0;
int d=1; 
frase = (char*)malloc(sizeof(char)); 

printf("Insert phrase: ");
while(c != '\n') { 
    c = getc(stdin);     
    frase = (char*)realloc(frase,d*sizeof(char)); //dynamic allocation
    frase[i] = c;
    d++;
    i++;
}

//at the end i terminate the string
frase[i]='\0';
printf("\nInserted phrase: %s\n",frase);

// here I start the reversal, first of all I separate all the words
char *pch;
char s[2] = " ";
int messl=0;

pch = strtok (frase,s);
printf ("%s\n",pch);
messl += 1 + strlen(pch);
printf ("Lung stringa = %d\n",messl);
char *message = (char*) malloc(messl);

while (pch != NULL) {
    pch = strtok (NULL, s);
    printf("%s\n",pch);
    messl += 1 + strlen(pch); //in the last cycle of the loop I get the error

}
//printf ("%s\n",message);
return 0;

Please [see why not to cast](http://stackoverflow.com/q/605845/2173917) the return value of `malloc()` and family in `C`. — Sourav Ghosh, Jul 31 '15 at 07:49
`frase[i]='\0';` writes past the end of the `realloc`'d space — M.M, Jul 31 '15 at 08:08

Sourav Ghosh · Accepted Answer · 2015-07-31T08:15:02.697

4

In your code.

 while(c != '\n')

at the very first iteration, c is uninitialised. It invokes undefined behaviour to use the value of an automatic local variable which has not been initialized explicitly.

getc() returns an int which , at times, may not fit into a char. Change the type of c to int.

That said, as you mentioned in your question, that you're getting segfault from strlen(), you need check for the non-NULL value of the passed pointer to strlen(). Add the NULL-check to pch immediately after tokenizing.

edited Jul 31 '15 at 08:15

answered Jul 31 '15 at 07:51

Sourav Ghosh

133,132
16
183
261

@user3318528 You're welcome . :-) BTW, you can consider [accepting](http://meta.stackexchange.com/q/5234) an answer that helped you. – Sourav Ghosh Jul 31 '15 at 15:02

score 2 · Answer 2 · answered Jul 31 '15 at 08:10

The main problem is:

while (pch != NULL) {
    pch = strtok (NULL, s);
    printf("%s\n",pch);
    messl += 1 + strlen(pch);

When strtok returns NULL, you go on to call printf and strlen on it. You need to immediately test pch upon calling strtok. For example the loop structure could be:

while ( (pch = strtok(NULL, s)) != NULL ) {

There are various other problems too, as other answerers/commentors have noted.

Segmentation fault when using strlen on user input string

2 Answers2