implementing cd system call using C- if condition

Question

Here is code that implements the cd system call using C. The problem with this code is that it's not entering the if condition if(strcmp(buffer,"cd") == 0) and I can't understand why.

#include<sys/stat.h>
#include <stdio.h>
#include <unistd.h>
#include <string.h>
#include<dirent.h>
#include<error.h>

#define BUFFERSIZE 20
int main(){

char *args[80]; 
char buffer[BUFFERSIZE];
char *prompt = "OS";
char *a = ">";
printf("%s%s",prompt,a); 
fgets(buffer, BUFFERSIZE, stdin);  

char *tok; 
tok = strtok (buffer," ");


while(buffer != NULL){ 
   buffer[strlen(buffer)-1] = '\0';  
   pid_t pid;
   pid = fork();
   if(pid < 0){
      fprintf(stderr, "Fork failed");
      return 1;
   }
   else if(pid == 0){

       if(strcmp(buffer,"cd") == 0){
         tok = strtok(NULL,"\n");
         cd(tok);
       }
       printf("%s%s",prompt,a); 
       fgets(buffer, BUFFERSIZE, stdin);
   }
   else{
     wait(NULL);
   }
}
return 0;
}


int cd(char *pth){
   char path[1000];
   strcpy(path,pth);

   static char *prompt = "OS";
   static char *a = ">";
   char *token;

   char cwd[256]; 
   getcwd(cwd,sizeof(cwd));

   strcat(cwd,"/"); 
   strcat(cwd,path);
   chdir(cwd);    

   printf("%s-%s%s",prompt,path,a);
   return 0;
  }

Q: Have you looked at the value of "temp" in your debugger of choice? Is it in fact equal to "cd"? Here's a good tutorial for "gdb": http://www.cs.cmu.edu/~gilpin/tutorial/ — paulsm4, Apr 18 '13 at 23:35
1) `buffer[strlen(buffer)-1] = '\0';` Bad habit, IMHO. strlen() *could* return zero. 2) `tok = strtok (temp," ");` tok shadows another `tok` — wildplasser, Apr 18 '13 at 23:36
`cd` *can't* be a standalone program, it has to be a shell builtin. What exactly are you trying to do? — Carl Norum, Apr 18 '13 at 23:44
`cd` is a shell command; the system call is `chdir`. And your code doesn't *implement* `chdir`, it merely calls it. — Keith Thompson, Apr 18 '13 at 23:47
Furthermore, `fgets()` returns the number of characters read *and* guarantees to NULL-terminate the buffer. Might also want to consider whether `while (buffer!=NULL)` can ever evaluate false, with `buffer` being on the stack? — marko, Apr 18 '13 at 23:47
actually what i am trying to do is that i am implementing cd(system call) using c programming. that is when the c program runs . it prints a prompt. user enter "cd XXX" and the C program then changes present directory to that directory(in this case XXX). — Urvah Shabbir, Apr 19 '13 at 00:01
@wildplasser. i didnt get what you meant by overshadowing another tok. — Urvah Shabbir, Apr 19 '13 at 00:03
@KeithThompson. yes my code isnt implementing chdir. it is implementing cd using in C using commands like chdir and getcwd etc. — Urvah Shabbir, Apr 19 '13 at 00:04
@Marko. yeah i have been facing problem exiting the program. coz while condition is not getting false. what can i replace NULL with inorder to get out of the loop? should equate it to some variable? like this int should_run == 1; while(should_run){} and to get out of the loop set should_run = -1 or is there a better way to do it? — Urvah Shabbir, Apr 19 '13 at 00:07
@CarlNorum. cd is not a stand alone program it is a function. which i am calling in main function of my c program. — Urvah Shabbir, Apr 19 '13 at 00:09
@user128806: Your title refers to "cd system call". `cd` is not a system call. That's my point. — Keith Thompson, Apr 19 '13 at 00:11
@user128806: there are two instances of `tok`: one outside the loop(never used) and one inside. That's the point where the confusion starts; not for the compiler (though it *could* warn), but for me. — wildplasser, Apr 19 '13 at 00:27
@wildplasser..ooops sorry.. i have posted the editted code... but it still doesnt set if condition true..Y? — Urvah Shabbir, Apr 19 '13 at 00:45
The if condition is never gonna get true because its in the child process! and you have not implemented any logic to pass the value to the child process. Further more why do you need a child process for this? — bikram990, May 07 '13 at 09:14
@bikram990 A child process is a copy of the parent process. All the variables are copied automatically, you don't need to pass them. — Barmar, May 08 '15 at 19:27

bikram990 · Accepted Answer · 2016-02-15T09:15:53.843

Have updated the logic after suggestions from others.

There is no need for a child process here. If you want multitasking then use threads. Child process may be required for process running in background.

The following program is working for me:

#include <stdio.h>

#include <sys/stat.h>
#include <stdio.h>
#include <unistd.h>
#include <string.h>
#include <dirent.h>
//#include <error.h>

int hasPrefix(char const *, char const *);
int cd(char *pth);

#define BUFFERSIZE 200
int main(){

    char buffer[BUFFERSIZE];
    char *prompt = "OS";
    char *a = ">";

    char *tok;
    tok = strtok (buffer," ");


    while(buffer != NULL){
        bzero(buffer, BUFFERSIZE);
        printf("%s%s",prompt,a);
        fgets(buffer, BUFFERSIZE, stdin);
        if(hasPrefix(buffer,"cd") == 0){
            tok = strchr(buffer,' '); //use something more powerful
            if(tok) {
                char *tempTok = tok + 1;
                tok = tempTok;
                char *locationOfNewLine = strchr(tok, '\n');
                if(locationOfNewLine) {
                    *locationOfNewLine = '\0';
                }
                cd(tok);
            }
        }else{
            system("ls"); //for testing the CWD/PWD
        }
    }
    return 0;
}

int hasPrefix(char const *p, char const *q)
{
    int i = 0;
    for(i = 0;q[i];i++)
    {
        if(p[i] != q[i])
            return -1;
    }
    return 0;
}

int cd(char *pth){
    char path[BUFFERSIZE];
    strcpy(path,pth);

    char cwd[BUFFERSIZE];
    if(pth[0] != '/')
    {// true for the dir in cwd
        getcwd(cwd,sizeof(cwd));
        strcat(cwd,"/");
        strcat(cwd,path);
        chdir(cwd);
    }else{//true for dir w.r.t. /
        chdir(pth);
    }

    return 0;
}

What's wrong with the standard `strcmp()` that requires writing his own. — Barmar, May 08 '15 at 19:29
You should not have suggested using a homebrew `mystrcmp`, and you've inadvertently demonstrated just why. The standard `strcmp` has nothing wrong here, while yours does. It has two bugs: 1. If `q` is an empty string (`""`), you will always report equality and 2. If the string `p` has the string `q` as a prefix but has trailing additional characters, it will report equality. It would have been better to [chop off the newline from `fgets()` with this one-liner](http://stackoverflow.com/a/28462221/2809095). — Iwillnotexist Idonotexist, May 09 '15 at 16:27
@Barmar Its been 1 year since I looked on this. I don't remember why I said it. If `strcmp` is right for you then you can use it. — bikram990, May 11 '15 at 06:29
@DanielJour thanks for pointing out. I've updated the answer now. — bikram990, Feb 15 '16 at 09:17
@Barmar Actaully I wanted a `hasPrefix` method and named the method as `mystrcmp` and blamed the standard `strcmp`. Now realizing that How foolish I was to blame standard `strcmp`. — bikram990, Feb 15 '16 at 09:20

score 3 · Answer 2 · answered May 11 '15 at 22:51

Use

...
if(strncmp(buffer,"cd",2) == 0){
...

instead. It is good for comparing prefixes of arbitrary length. It's also puts a limit on string size. No need to build your own comparison routine.

You have other problems elsewhere in the code, but those can be addressed separately.

score 1 · Answer 3 · answered May 08 '15 at 19:32

1

I think the problem is because of this line:

buffer[strlen(buffer)-1] = '\0';

This replaces the last character of buffer with a null character. So if buffer contained "cd", it now contains just "c" (since the null character is the string terminator in C).

There doesn't seem to be any need for this statement, just remove it.

answered May 08 '15 at 19:32

Barmar

741,623
53
500
612

Just stumbled upon this, and this is really the only correct answer. – Daniel Jour Feb 11 '16 at 18:54

implementing cd system call using C- if condition

3 Answers3