Issue
I am currently doing the MIT opencourseware OS course (https://ocw.mit.edu/courses/6-828-operating-system-engineering-fall-2012/) and I am struggling with the first class assignment which is implementing your own small shell.
My code is the following:
#include <stdlib.h>
#include <unistd.h>
#include <stdio.h>
#include <fcntl.h>
#include <string.h>
#include <assert.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/wait.h>
#include <stdint.h>
// Simplifed xv6 shell.
#define MAXARGS 10
// All commands have at least a type. Have looked at the type, the code
// typically casts the *cmd to some specific cmd type.
struct cmd {
int type; // ' ' (exec), | (pipe), '<' or '>' for redirection
};
struct execcmd {
int type; // ' '
char *argv[MAXARGS]; // arguments to the command to be exec-ed
};
struct redircmd {
int type; // < or >
struct cmd *cmd; // the command to be run (e.g., an execcmd)
char *file; // the input/output file
int mode; // the mode to open the file with
int fd; // the file descriptor number to use for the file
};
struct pipecmd {
int type; // |
struct cmd *left; // left side of pipe
struct cmd *right; // right side of pipe
};
int fork1(void); // Fork but exits on failure.
struct cmd *parsecmd(char*);
// Execute cmd. Never returns.
void
runcmd(struct cmd *cmd)
{
int p[2], r;
struct execcmd *ecmd;
struct pipecmd *pcmd;
struct redircmd *rcmd;
if(cmd == 0)
exit(0);
switch(cmd->type){
default:
fprintf(stderr, "unknown runcmd\n");
exit(-1);
case ' ':
ecmd = (struct execcmd*)cmd;
if(ecmd->argv[0] == 0)
exit(0);
if (strcmp(ecmd->argv[0], "ls") == 0) {
execvp("ls", ecmd->argv);
fprintf(stderr, "execvp ls failed\n");
}
if (strcmp(ecmd->argv[0], "echo") == 0) {
perror("This is from echo:\n");
execvp("echo", ecmd->argv);
fprintf(stderr, "execvp failed\n");
}
if (strcmp(ecmd->argv[0], "grep") == 0) {
//perror("This is from grep:\n");
execvp("grep", ecmd->argv);
fprintf(stderr, "execvp failed\n");
}
if (strcmp(ecmd->argv[0], "cat") == 0) {
//perror("This is from grep:\n");
execvp("cat", ecmd->argv);
fprintf(stderr, "execvp failed\n");
}
break;
case '>':
case '<':
rcmd = (struct redircmd*)cmd;
fprintf(stderr, "redir not implemented\n");
// Your code here ...
runcmd(rcmd->cmd);
break;
case '|':
pcmd = (struct pipecmd*)cmd;
struct execcmd *left = (struct execcmd*)pcmd->left;
struct execcmd *right = (struct execcmd*)pcmd->right;
int i;
int pd[2];
pipe(pd);
int pid1 = fork1();
int pid2 = fork1();
if(!pid1) {
// close(STDOUT_FILENO);
// close(pd[0]);
execvp(left->argv[0], left->argv);
dup2(pd[1], 1);
exit(0);
}
if(!pid2) {
// close(STDIN_FILENO);
// close(pd[1]);
dup2(pd[0], 0);
execvp(right->argv[0], right->argv);
exit(0);
}
if(pid1 > 0) {
int status2;
int status;
if(waitpid(pid1, &status, 0) == -1){
perror("waitpid1");
}
if(waitpid(pid2, &status2, 0) == -1) {
perror("waitpid2");
};
// close(pd[0]);
// close(pd[1]);
}
break;
}
}
int
getcmd(char *buf, int nbuf)
{
if (isatty(fileno(stdin)))
fprintf(stdout, "$ ");
memset(buf, 0, nbuf);
fgets(buf, nbuf, stdin);
if(buf[0] == 0) // EOF
return -1;
return 0;
}
int
main(void)
{
static char buf[100];
int fd, r;
// Read and run input commands.
while(getcmd(buf, sizeof(buf)) >= 0){
if(buf[0] == 'c' && buf[1] == 'd' && buf[2] == ' '){
// Clumsy but will have to do for now.
// Chdir has no effect on the parent if run in the child.
buf[strlen(buf)-1] = 0; // chop \n
if(chdir(buf+3) < 0)
fprintf(stderr, "cannot cd %s\n", buf+3);
continue;
}
if(fork1() == 0)
runcmd(parsecmd(buf));
wait(&r);
}
exit(0);
}
int
fork1(void)
{
int pid;
pid = fork();
if(pid == -1)
perror("fork");
return pid;
}
struct cmd*
execcmd(void)
{
struct execcmd *cmd;
cmd = malloc(sizeof(*cmd));
memset(cmd, 0, sizeof(*cmd));
cmd->type = ' ';
return (struct cmd*)cmd;
}
struct cmd*
redircmd(struct cmd *subcmd, char *file, int type)
{
struct redircmd *cmd;
cmd = malloc(sizeof(*cmd));
memset(cmd, 0, sizeof(*cmd));
cmd->type = type;
cmd->cmd = subcmd;
cmd->file = file;
cmd->mode = (type == '<') ? O_RDONLY : O_WRONLY|O_CREAT|O_TRUNC;
cmd->fd = (type == '<') ? 0 : 1;
return (struct cmd*)cmd;
}
struct cmd*
pipecmd(struct cmd *left, struct cmd *right)
{
struct pipecmd *cmd;
cmd = malloc(sizeof(*cmd));
memset(cmd, 0, sizeof(*cmd));
cmd->type = '|';
cmd->left = left;
cmd->right = right;
return (struct cmd*)cmd;
}
// Parsing
char whitespace[] = " \t\r\n\v";
char symbols[] = "<|>";
int
gettoken(char **ps, char *es, char **q, char **eq)
{
char *s;
int ret;
s = *ps;
while(s < es && strchr(whitespace, *s))
s++;
if(q)
*q = s;
ret = *s;
switch(*s){
case 0:
break;
case '|':
case '<':
s++;
break;
case '>':
s++;
break;
default:
ret = 'a';
while(s < es && !strchr(whitespace, *s) && !strchr(symbols, *s))
s++;
break;
}
if(eq)
*eq = s;
while(s < es && strchr(whitespace, *s))
s++;
*ps = s;
return ret;
}
int
peek(char **ps, char *es, char *toks)
{
char *s;
s = *ps;
while(s < es && strchr(whitespace, *s))
s++;
*ps = s;
return *s && strchr(toks, *s);
}
struct cmd *parseline(char**, char*);
struct cmd *parsepipe(char**, char*);
struct cmd *parseexec(char**, char*);
// make a copy of the characters in the input buffer, starting from s through es.
// null-terminate the copy to make it a string.
char
*mkcopy(char *s, char *es)
{
int n = es - s;
char *c = malloc(n+1);
assert(c);
strncpy(c, s, n);
c[n] = 0;
return c;
}
struct cmd*
parsecmd(char *s)
{
char *es;
struct cmd *cmd;
es = s + strlen(s);
cmd = parseline(&s, es);
peek(&s, es, "");
if(s != es){
fprintf(stderr, "leftovers: %s\n", s);
exit(-1);
}
return cmd;
}
struct cmd*
parseline(char **ps, char *es)
{
struct cmd *cmd;
cmd = parsepipe(ps, es);
return cmd;
}
struct cmd*
parsepipe(char **ps, char *es)
{
struct cmd *cmd;
cmd = parseexec(ps, es);
if(peek(ps, es, "|")){
gettoken(ps, es, 0, 0);
cmd = pipecmd(cmd, parsepipe(ps, es));
}
return cmd;
}
struct cmd*
parseredirs(struct cmd *cmd, char **ps, char *es)
{
int tok;
char *q, *eq;
while(peek(ps, es, "<>")){
tok = gettoken(ps, es, 0, 0);
if(gettoken(ps, es, &q, &eq) != 'a') {
fprintf(stderr, "missing file for redirection\n");
exit(-1);
}
switch(tok){
case '<':
cmd = redircmd(cmd, mkcopy(q, eq), '<');
break;
case '>':
cmd = redircmd(cmd, mkcopy(q, eq), '>');
break;
}
}
return cmd;
}
struct cmd*
parseexec(char **ps, char *es)
{
char *q, *eq;
int tok, argc;
struct execcmd *cmd;
struct cmd *ret;
ret = execcmd();
cmd = (struct execcmd*)ret;
argc = 0;
ret = parseredirs(ret, ps, es);
while(!peek(ps, es, "|")){
if((tok=gettoken(ps, es, &q, &eq)) == 0)
break;
if(tok != 'a') {
fprintf(stderr, "syntax error\n");
exit(-1);
}
cmd->argv[argc] = mkcopy(q, eq);
argc++;
if(argc >= MAXARGS) {
fprintf(stderr, "too many args\n");
exit(-1);
}
ret = parseredirs(ret, ps, es);
}
cmd->argv[argc] = 0;
return ret;
}
The part I am struggling with is the pipe implementation:
case '|':
pcmd = (struct pipecmd*)cmd;
struct execcmd *left = (struct execcmd*)pcmd->left;
struct execcmd *right = (struct execcmd*)pcmd->right;
int i;
int pd[2];
pipe(pd);
int pid1 = fork1();
int pid2 = fork1();
if(!pid1) {
// close(STDOUT_FILENO);
// close(pd[0]);
execvp(left->argv[0], left->argv);
dup2(pd[1], 1);
exit(0);
}
if(!pid2) {
// close(STDIN_FILENO);
// close(pd[1]);
dup2(pd[0], 0);
execvp(right->argv[0], right->argv);
exit(0);
}
if(pid1 > 0) {
int status2;
int status;
if(waitpid(pid1, &status, 0) == -1){
perror("waitpid1");
}
if(waitpid(pid2, &status2, 0) == -1) {
perror("waitpid2");
};
// close(pd[0]);
// close(pd[1]);
}
break;
I am trying to execute echo "hahaha" | grep "h"
without getting any output. Both echo and grep work individually.
I checked that left->argv[0]
and right->argv[0]
work as well.
Does anybody have an idea what is going on?
PS: I am only implementing so that you can execute one pipe. I am aware of that I would need a loop if i wanted to make it possible to execute more than 2 piped commands
Solution
A little rewrite of your code is below. Main issues were:
- dup2() is used after execvp, should be done before.
- fork1() is used to early for "pid2", results in to many forks.
- parent should always close pd[]
- childs shoud close the unused pd[] part.
- command arguments given to execvp are as-is [per your test].
To test the your and the below code, you should not use surrounding quotes for the grep command argument. (That is a seperate parsing issue/challenge)
To test:
$ echo hahahaha | grep a
The rewrite:
case '|':
pcmd = (struct pipecmd*)cmd;
struct execcmd *left = (struct execcmd*)pcmd->left;
struct execcmd *right = (struct execcmd*)pcmd->right;
int i;
int pd[2];
pipe(pd);
int pid1 = fork1();
if(!pid1) {
// close(STDOUT_FILENO);
close(pd[0]);
dup2(pd[1], 1);
execvp(left->argv[0], left->argv);
exit(0);
}
int pid2 = fork1();
if(!pid2) {
// close(STDIN_FILENO);
close(pd[1]);
dup2(pd[0], 0);
execvp(right->argv[0], right->argv);
exit(0);
}
close(pd[0]);
close(pd[1]);
int status2;
int status;
if(waitpid(pid1, &status, 0) == -1){
perror("waitpid1");
}
if(waitpid(pid2, &status2, 0) == -1) {
perror("waitpid2");
};
break;
Good luck
Answered By - S_IROTH Answer Checked By - Marilyn (WPSolving Volunteer)