-4

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

  • 1
    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
  • 1
    @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
  • 1
    @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
  • 1
    @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

6 Answers6

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," ");
    }
}
NPE
  • 486,780
  • 108
  • 951
  • 1,012
  • 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
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);
}
Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
  • 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
  • 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
2

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.

Rohan
  • 52,392
  • 12
  • 90
  • 87
1

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).

Sebastian Mach
  • 38,570
  • 8
  • 95
  • 130
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," ");
    }
}
0

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

  • 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