Segmentation Fault in C function

Question

I am trying to develop a basic shell. For that shell I need a C function to parse a string. As i am new to C I tried to develop a basic function and it gives me a segmentation fault error. Please tell me what I am missing.

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


void parse(char *msg);
int main()
{
    char *msg =  "This is a message";
    parse(msg);
}

void parse(char *msg){
    char *mm;
    mm = msg;

    char *tok;
    tok = strtok(mm," ");
    while(tok == NULL){
        tok = strtok(NULL," ");
            printf("%s \n",tok);
    }
}

Error Message (Runtime)

Segmentation fault (core dumped)

Thanks in advance

How many times you guys are going to ask this very same question **and** at the same time not RTFM? String literals are constants, there is **no friggin' way** you can modify their contents. — , Apr 09 '13 at 05:48
(For the exact same reason, **`const`** `char *msg = "foo";` — , Apr 09 '13 at 05:49
@H2CO3, Perhaps it is time to edit the C tag wiki similar questions to add questions related to [undefined behaviour](http://stackoverflow.com/questions/4176328/undefined-behavior-and-sequence-points) and [sockets](http://stackoverflow.com/questions/307692/simplest-way-to-open-and-use-a-socket-in-c) since they seem to be over-asked? — Anish Ramaswamy, Apr 09 '13 at 05:52
@AnishRam That's a constructive suggestion. (Unfortunately, people who ask these types of questions don't tend to read the tag wikis either.) — , Apr 09 '13 at 05:53
@H2CO3, Hmm. Good point. (Off to brainstorm new feature request :D) — Anish Ramaswamy, Apr 09 '13 at 05:54
@H2CO3: to be fair to the OP, that's not the only bug in their code that could lead to a segfault ;-) — NPE, Apr 09 '13 at 05:55
@NPE: what else leads to a segfault? I can see bugs that lead to no output appearing, but that's a very different problem from a segfault. — Jonathan Leffler, Apr 09 '13 at 05:56
@JonathanLeffler: I meant the NULL pointer business in `parse()`. However, this would require the caller to supply a different input. In any case, see the smiley face. — NPE, Apr 09 '13 at 05:59
@NPE Well, while I couldn't immediately grasp that, I'm sure there will be some other parts of the code which suffer from similar problems (but that's why the C-FAQ has been created, and in my opinion, this question is 1. lacks research effort, 2. (so) it's a dupe, 3. thusly it's not suited for Stack Overflow.) — , Apr 09 '13 at 05:59
@H2CO3: Oh, I am not arguing that this is a particularly good question. It definitely lacks research effort. That said, it is clear, complete and has nearly-working code, which makes it a better question than much of what I see asked here. Also, I suspect you might have overlooked the smiley face at the end of my other comment. :-) — NPE, Apr 09 '13 at 06:07
@NPE Nope, don't worry ;-) And yes, you are right in that it basically comes with a SSCCE, and that's rare. — , Apr 09 '13 at 06:08

NPE · Answer 1 · 2013-04-09T05:51:23.963

5

msg points to a string literal, and you are attempting to modify it. In C, modifying string literals is undefined behaviour (in practice, compilers often place them in read-only memory).

To fix, turn msg into an array:

int main()
{
    char msg[] =  "This is a message";
    parse(msg);
}

Also, there are a couple of issues with your while loop:

1) the condition is the wrong way round;
2) the second strtok() call should appear after the printf().

void parse(char *msg){
    char *mm = msg;
    char *tok = strtok(mm, " ");
    while (tok) {
        printf("%s \n",tok);
        tok = strtok(NULL," ");
    }
}

edited Apr 09 '13 at 05:51

answered Apr 09 '13 at 05:46

NPE

464,258
100
912
987

1

@phresnel **The fact** that modifying them results in UB permits the compiler to place them in ROM. The original wording was fine too. – Apr 09 '13 at 05:54
1

@H2CO3: That's exactly what I meant by the original wording, thanks. I've changed it as it seemed not clear enough. – NPE Apr 09 '13 at 05:56
@H2CO3: Or let's say, the wording was not totally unambiguous to at least one person ("what follows from what"). I removed my comment. +1 btw. – Sebastian Mach Apr 09 '13 at 05:57
@phresnel I see it, thanks. – Apr 09 '13 at 05:57

score 4 · Answer 2 · answered Apr 09 '13 at 05:46

4

You can't reliably modify a string literal; they are often readonly (and in your case, clearly are readonly). An attempt to modify a string literal invokes undefined behaviour, which is always a Bad Thing™!

Use:

int main(void)
{
    char msg[] = "This is a message";
    parse(msg);
}

answered Apr 09 '13 at 05:46

Jonathan Leffler

698,132
130
858
1,229

how did you write the trade mark symbol?can i write a superscript or subscript?i know its off topic but got curious. – Koushik Shetty Apr 09 '13 at 05:48
@Koushik You can always just copy the character from somewhere like [Wikipedia](http://en.wikipedia.org/wiki/Trademark_symbol). Also, for questions about things like formatting and such about Stack Overflow, there's http://meta.stackoverflow.com/. – ajp15243 Apr 09 '13 at 05:51
@Koushik: On a Mac keyboard, Option 2. Or use either the keyboard viewer or character viewer. Elsewhere, there's usually a way, not always obvious. The Unicode code point is U+2122; you may be able to do something with the Alt key and ... end up typing 2122. – Jonathan Leffler Apr 09 '13 at 05:51
@ajp15243 yeah thats one option but his response was quick and so dint think he would have copied from somewhere.:-) – Koushik Shetty Apr 09 '13 at 05:54
@JonathanLeffler thanks. – Koushik Shetty Apr 09 '13 at 05:56
I must admit, I am slightly jealous of that Option + 2 entry method for it. – ajp15243 Apr 09 '13 at 05:56
@ajp15243 right?:-) me too. – Koushik Shetty Apr 09 '13 at 05:57
1

Note you can also "fake" a ^TM-thingy using some of the allowed HTML. edit: Obviously, in comments it does not work. – Sebastian Mach Apr 09 '13 at 05:59
1

Anyone want to make a question of it? How to enter arbitrary Unicode characters on a Windows or Linux or Mac keyboard? – Jonathan Leffler Apr 09 '13 at 06:00
@JonathanLeffler: It seems it hasn't been asked yet (i.e. it is not a first-result on google, yet). Take the opportunity, otherwise I might steal your idea and ask my first superuser question in the next hour :D – Sebastian Mach Apr 09 '13 at 06:06
Be my guest — link to it when you've asked it. – Jonathan Leffler Apr 09 '13 at 06:08
@phresnel HTML sounds like a lot of work:-) it would be so cool to just get that in couple of keypress,if not for a keypress. – Koushik Shetty Apr 09 '13 at 06:08
@Koushik: Though it consistently works accross operating systems, distros, and browsers. – Sebastian Mach Apr 09 '13 at 06:10
@phresnel there is no denying of that fact. – Koushik Shetty Apr 09 '13 at 06:11

score 2 · Answer 3 · answered Apr 09 '13 at 05:47

Your definition:

char *msg =  "This is a message";

makes msg as constant char string which cannot be modified. But strtok modifies it.

You may want to change it to

char *msg =  strdup("This is a message");

Don't forget to free the pointer after you are done.

Sebastian Mach · Answer 4 · 2013-04-09T06:01:57.360

Perhaps instead of

while(tok == NULL)

you meant

while(tok != NULL)

or just

while (tok) // <- because in C, conditions are always compared to 0

. However, what gives you the segmentation fault is that strtok modifies the passed string, which is why it takes a non-const pointer to char (see http://linux.die.net/man/3/strtok). Therefore, because you are passing a pointer to a string literal, which are not modifiable, you receive a segmentation fault (which is your luck; this bug might also slip through QA and go production if you are unlucky).

Harsh Vardhan Sharma · Answer 5 · 2013-04-09T06:06:53.353

1

Go with it.........

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


void parse(char *msg);
int main()
{
    char msg[] =  "This is a message";
    parse(msg);
}

void parse(char *msg){
    char *mm;
    mm = msg;

    char *tok;
    tok = strtok(mm," ");
    while(tok != NULL){
        printf("%s \n",tok);
    tok = strtok(NULL," ");
    }
}

edited Apr 09 '13 at 06:06

answered Apr 09 '13 at 05:54

Harsh Vardhan Sharma

21
2

Sorry, but this is not helpful at all. My recommendation is to not post "Try This"-answers (see also my profile page). – Sebastian Mach Apr 09 '13 at 06:01
Your edit doesn't make this any more helpful. How about adding some explanations? – Sebastian Mach Apr 09 '13 at 06:07
I am not getting what clarification u want regarding this? – Harsh Vardhan Sharma Apr 09 '13 at 06:11
1

You gave the asker a complete solution. This may work now. But the askers possibly learns nothing from it for the future. There is a saying: "Give a man a fish, you feed him for a day; teach a man to fish, you feed him for life." – Sebastian Mach Apr 09 '13 at 06:15
1

Thanks for your Suggestion.....really helps – Harsh Vardhan Sharma Apr 09 '13 at 06:17
You are very Welcome. – Sebastian Mach Apr 09 '13 at 06:22

score 0 · Answer 6 · answered Apr 09 '13 at 05:48

0

how about change tok==NULL to tok!=NULL

answered Apr 09 '13 at 05:48

Minseong Park

1

But this answer was already redundant when you posted it, which is why Stack Overflow notifies you of new answers when pressing the submit-button. – Sebastian Mach Apr 09 '13 at 05:52

Segmentation Fault in C function

6 Answers6