bdoetsch@Kaladin:~/Documents/School$ ./shell
Shell(pid = 6955) 1> ls
command: ls
argv[i] = ls
argv[i] = ./shell
Parent says 'child process has been forked with pid=6956'
./shell
Parent says 'wait() returned so the child with pid=-1 is finished'
Shell(pid = 6955) 2>
This is homework I am working on, but I am a little stumped.
Hi, I am trying to write a shell program and I don't understand why getline is getting the preceding line of input.
plus the program will execute 'ls -al, 'pwd and a few others. but not just ls.
#include <sys/types.h>
#include <stdio.h>
#include <unistd.h>
#include <stdlib.h>
#include <string.h>
int tokenizer(char *str,char **argv);
void doCommand(char **argv);
int main()
{
pid_t pid;
char *command;
int Num_bytes_read;
size_t nbytes = 60;
int NumCommand = 0;
char **argv;
int NumOfArgs;
command = (char *) malloc (nbytes + 1);
command = NULL;
while(1)
{
NumCommand++;
printf("Shell(pid = %d) %d> ",getpid(),NumCommand);
fflush(stdin);
Num_bytes_read = getline(&command,&nbytes,stdin);
printf("command: %s",command);
if(Num_bytes_read == -1)
{
printf("\n ERRor\n");
}
else
{
int x = tokenizer(command,argv);
int i;
for (i = 0; i < (x+1); ++i)
printf ("argv[i] = %s\n",argv[i]);
//if (strcmp(argv[0], "quit")){
//break;
//}
doCommand(argv);
}
}
return 0;
}
int tokenizer(char *str, char **argv)
{
//const char s[2] = " ";
char ** res = NULL;
char * p = strtok (str, " \n");
int n_spaces = 0, i;
while (p)
{
res = realloc (res, sizeof (char*) * ++n_spaces);
if (res == NULL)
exit (-1); /* memory allocation failed */
res[n_spaces-1] = p;
argv[n_spaces-1]= res[n_spaces-1];
p = strtok (NULL, " \n");
}
// realloc one extra element for the last NULL
res = realloc (res, sizeof (char*) * (n_spaces+1));
res[n_spaces] = 0;
argv = res;
return n_spaces;
}
void doCommand(char **argv)
{
pid_t pid;
pid_t cpid; /* Pid of child to be returned by wait. */
int fd[2]; // dual pipeline
int status; /* Exit status of child. */
int nbytes;
int commandStatus;
pipe(fd);
pid = fork(); // Preceding with fork]
if (pid < 0)
{
printf("forking child process failed\n");
exit(1);
}
else if (pid == 0) // fork for the child
{
close(fd[0]); // close up reader side of pipe
cpid = getpid();
/* Send "string" through the output side of pipe */
write(fd[1], &cpid,sizeof(cpid));
//argv[0] = "ls";
//argv[1] = NULL;
commandStatus = execvp(*argv, argv);
if (commandStatus < 0) /* execute the command */
{
printf("Try again, command failed\n");
exit(1);
}
}
else if (pid > 1) // fork for the parent
{
close(fd[1]);
/* Read in the child pid from the pipe */
nbytes = read(fd[0], &cpid , sizeof(cpid));
printf("Parent says 'child process has been forked with pid=%ld'\n",(long)cpid);
wait(NULL);
cpid = wait(&status); /* wait for completion */
printf("Parent says 'wait() returned so the child with pid=%ld is finished'\n",(long)cpid);
}
}
The problem is that argv
is not really initialized, because you are altering the local pointer in tokenizer()
, instead of the pointer in main()
.
What you have to do is pass argv
's address instead of the pointer itself, and then change tokenizer()
like this
int tokenizer(char *str, char ***argv)
{
char ** res = NULL;
char * p = NULL;
int n_spaces = 0;
if ((str == NULL) || (argv == NULL))
return 0;
p = strtok (str, " \n");
*argv = NULL;
while (p)
{
res = realloc (*argv, sizeof(char *) * ++n_spaces);
if (res == NULL)
{
free(*argv);
return 0;
}
res[n_spaces - 1] = p;
*argv = res;
p = strtok (NULL, " \n");
}
res = realloc (*argv, sizeof(char *) * (n_spaces + 1));
if (res == NULL)
{
free(*argv);
return 0;
}
res[n_spaces] = 0;
*argv = res;
return n_spaces;
}
and in main
int x = tokenizer(command, &argv);
/* ^ address of argv */
you never checked the last realloc
too, so I fixed some of other things I believe you was doing a little bit unsafe.
exit()
on malloc
/realloc
failure is not really necessary in your case, you can just free()
the already alloced pointer and just return 0
, then handle that in main()
.
Also, I recommend the usage of the const
qualifier wherever it makes sense, for example
void doCommand(const char *const *argv)
And also, don't fflush()
stdin
it's undefined behavior.