Character arrays, and pointers how to use strtok and strcmp

Question

I'm writing a linux shell for my operating systems class. I've knocked out the majority of it but I'm stuck on simple string comparisons. I've everything I can think of. strcmp should take in \0 terminated strings and return 0 for equal but that doesn't seem to be working and even stepping through the array and checking each char isn't working either. I currently have cmd[0] in the strcmp I know thats not right it needs to be null terminated but I've tried using strcpy and strcat \0 to another string. If someone could point out my mistake it would be much appreciated.

//Matthew Spiers
//CSC306

#include <string.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <iostream>
#include <unistd.h>
#include <stdlib.h>

using namespace std;

void ckCmd(char dir[]);
int main(){

    pid_t pid;
    char cdstr[4] = "cd";
    char str[50];
    char *cmd[3];
    char *pstr;
    char temp[50];
    char dir[50] = "/bin/";
    while(1){
        pid = fork();
        if(pid < 0){
            fprintf(stdout, "Fork Failed");
        }
        else if(pid == 0){
            fprintf(stdout, "\e[36m306.SH>\e[0m");
            fgets(str, 50, stdin);  
            for(int i =0; i<50; i++){
                if(str[i] == '\n'){
                    str[i] = '\0';
                }
            }
            strcpy(temp, str); // Make a copy of original string
            cmd[0] = strtok(str, " ");
            for(int i =1; i<3; i++){
                cmd[i] = strtok(NULL, " ");
            }
            strcat(dir, cmd[0]);

            cout << cmd[0];

                    pstr = strtok(temp, " ");  //pull out only first token
            //Change Directory
            if(!strcmp(pstr, "cd")){ //if first token is cd
                //either branch to a routine just change directory
                //ie branch and change directory
            }
            //ckCmd(temp);

            execlp(dir, cmd[0], cmd[1], cmd[2], NULL);
            _exit(0);
        }
        else{
            wait(NULL);
        }
    }
}

void ckCmd(char str[]){
    char *p;
    p = strtok(str, " ");
    if(p[0] == 'c'){
        chdir("new");
    }
}

    enter code here

Even if the functions you are talking about are pure C, the inclusion indicates that you are using C++, so I have removed the tag, retag if you deem appropriate. Once you are going to use C++, I would advice to use `std::string` and avoid using `strtok` and `strcmp` altogether, unless that is part of the requirements for the exercise. — David Rodríguez - dribeas, Sep 16 '10 at 15:25
It's full of errors (`iostream`, `using`, `namespace`, `cout`) — pmg, Sep 16 '10 at 15:49
@Nikko: I don't think that the question is on C, there are some C++ exclusive things: `#include `, `using namespace std;`, `cout` — David Rodríguez - dribeas, Sep 16 '10 at 16:43
@David :just standard output that could just be replaced with C equivalent. — Nikko, Sep 17 '10 at 00:15

score 1 · Answer 1 · answered Sep 16 '10 at 15:40

1

strtok is not reentrant/thread-safe! You should use the RETURN-value from strtok:

p = strtok(str, " ");
    if(p[0] == 'c'){

cmd[0] = strtok(str, " ");
...
if(!strcmp(cmd[0], "cd")){

If p/cmd[0] is NULL, it will crash.

answered Sep 16 '10 at 15:40

user411313

3,930
19
16

should i just be using strok_r write my own code to parse the strings? – Matt Sep 16 '10 at 16:05
1

If you have a bugfree strtok_r version, take it, but isnt standard. If you dont have it, you can take my version on http://stackoverflow.com/questions/3375530/parse-tokens-from-a-string-c-programming-strtok-issue/3418673#3418673 – user411313 Sep 16 '10 at 19:34

Eli Bendersky · Answer 2 · 2010-09-16T19:29:11.617

0

What exactly isn't working? Can you show a smaller code sample?

The line:

strcat(dir, cmd[0]);

Are you aware that dir is the target here and not cmd[0]?

The line: !strcmp(cmd[0], "cd") by itself is correct, but it's unclear what exactly you're trying to do. Could you comment each group of line with your intentions?

Update: I suggest that you're trying too many things at once. To home in on your problem I recommend the following steps:

Understand how strtok works. It isn't very hard - read its manual and then try it in a separate file with some strings. This should give you a good idea.
Break parts of your code out and provide them with pre-set input (not from the user and without the forking). See where the behavior is as expected and where it drifts off.

edited Sep 16 '10 at 19:29

answered Sep 16 '10 at 15:35

Eli Bendersky

263,248
89
350
412

//Change Directory if(!strcmp(cmd[0], "cd")){ //Do Something //ie branch and change directory } ckCmd(temp); This part. everthing else works fine I also realize cmd[0] isn't goign to work because it doesn't have a \0. I tried: copying the original string to temp then stripping out first token and even adding the \0 after that not sure if strtok terminates with null. Also he was kinda vague in the requirements. One of the goals is to understand strtok() but he didn't implicitly say to use it. – Matt Sep 16 '10 at 15:44
I added some comments and changed cmd[0] – Matt Sep 16 '10 at 16:04

Character arrays, and pointers how to use strtok and strcmp

2 Answers2