I am creating my own shell in C using fork and execvp. I am parsing the cmd and it's arguments using strtok. Printing the parsed tokens confirms to me that I am indeed receiving all the arguments, and the shell generally works, though it is very basic of course because I am quite a noob. However, here are two typical scenarios when I run the shell which are baffling me, notice how the ls and pwd commands work the first time, but then begin to get these extra operands from somewhere:
➜ a2 ./shell
(user)># pwd
/home/user/ClionProjects/unix_programming/a2
(user)># ls -la
total 32
drwxrwxr-x 2 user user 4096 Mar 9 13:18 .
drwxrwxr-x 9 user user 4096 Mar 6 14:18 ..
-rwxrwxr-x 1 user user 13616 Mar 9 13:18 shell
-rw-rw-r-- 1 user user 3809 Mar 9 13:17 shell.c
-rw-rw-r-- 1 user user 545 Mar 9 12:58 shell.h
(user)># pwd
pwd: ignoring non-option arguments
/home/user/ClionProjects/unix_programming/a2
(user)>#
AND
➜ a2 ./shell
(user)># pwd
/home/user/ClionProjects/unix_programming/a2
(user)># ls -la
total 32
drwxrwxr-x 2 user user 4096 Mar 9 13:18 .
drwxrwxr-x 9 user user 4096 Mar 6 14:18 ..
-rwxrwxr-x 1 user user 13616 Mar 9 13:18 shell
-rw-rw-r-- 1 user user 3809 Mar 9 13:17 shell.c
-rw-rw-r-- 1 user user 545 Mar 9 12:58 shell.h
(user)># pwd
pwd: ignoring non-option arguments
/home/user/ClionProjects/unix_programming/a2
(user)>#
In the code below you can see that I store the arguments in tokens. The first token being the cmd filename itself.
I have tried memsetting tokens to all zeros, freeing it and re-mallocing it at the start of each iteration of the while loop. I have tried always resizing tokens to the number of args currently in it. I have done these things because I have become convinced that somehow, tokens is retaining content from previous commands that are for some reason being read by later commands. However, re-mallocing and reallocing tokens has not touched the problem, so I am now at a loss. Any advice or nudges in the right direction much appreciated.
shell.h
#ifndef UNIX_PROGRAMMING_SHELL_H
#define UNIX_PROGRAMMING_SHELL_H
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <errno.h>
#include <limits.h>
#include <signal.h>
int sig_int = 0;
void signal_handler(int sig_num);
void start_shell();
void prompt();
void change_dir(char *path, pid_t *pid, int *status);
void execute_cmd(const char *file_name, char *const *args);
void parse_cmd(char *cmd, char **tokens, size_t *index);
#endif //UNIX_PROGRAMMING_SHELL_H
shell.c
#include "shell.h"
void signal_handler(int sig_num) {
printf("\n"); // Do nothing when Ctrl-C is pressed
}
void prompt() {
/* Get current users username and display prompt */
char *user = getlogin();
printf("(%s)># ", (user==NULL)? "": user);
}
void change_dir(char *path, pid_t *pid, int *status) {
*pid = fork();
if (*pid == -1) {
printf("Error changing directory..\n");
} else if (*pid == 0) {
chdir(path);
} else {
*pid = wait(status);
if (*pid == -1) {
printf("%s\n", strerror(errno));
return;
}
}
}
void execute_cmd(const char *file_name, char *const *args) {
execvp(file_name, args);
}
/** parse commands into tokens... **/
void parse_cmd(char *cmd, char **tokens, size_t *index) {
char *tok;
const char *delim = " ";
*index = 0;
// TODO: realloc tokens, so it can start from 2 and build up as needed
tok = strtok(cmd, delim);
if (tok != NULL) {
tokens[*index] = tok;
(*index)++;
} else {
tokens[*index] = "\0";
return;
}
while ((tok = strtok(NULL, delim)) != NULL) {
tokens[*index] = tok;
(*index)++;
}
// for (size_t i = 0; i < *index; i++) {
// printf("arg[%zu]: %s\n", i, tokens[i]);
// }
printf("\n");
}
void start_shell() {
ssize_t c;
size_t cmd_size = 20;
size_t num_args = 5;
int *status = NULL;
pid_t pid;
char *cmd = (char *) malloc(sizeof(char)); /* command line input */
char **tokens = (char **) malloc(sizeof(char *) * num_args); /* command line input parsed into tokens */
size_t *index = (size_t *) malloc(sizeof(size_t)); /* number of tokens parsed */
if (tokens == NULL) {
printf("Error: Out of memory..");
exit(EXIT_FAILURE);
}
prompt();
/* main loop - get input, parse, process - until termination */
while ( (c = getline(&cmd, &cmd_size, stdin)) != EOF ) {
cmd[strcspn(cmd, "\n")] = '\0'; /* trim newline */
parse_cmd(cmd, tokens, index);
/* resize tokens to fit only it's current contents */
// if (*index < num_args && *index != 0 && *index != 1) {
tokens = realloc(tokens, *index);
// }
const char *file_name = tokens[0];
char *const *args = tokens;
/* If command is blank */
if ( (c = strcspn(file_name, "\n\r\0") == 0) ) {
tokens[0] = "\0";
prompt();
continue;
} else if ( (c = strcmp(file_name, "exit")) == 0 ) {
break;
} else if ( (c = strcmp(file_name, "cd")) == 0 ) {
if (*index == 1) { /* no path provided */
chdir(getenv("HOME"));
} else {
char *path = realpath(args[1], NULL);
if (path == NULL) {
printf("%s\n", strerror(errno));
break;
} else {
change_dir(path, &pid, status);
}
}
}
// fork here ... success: parent < pid .. child << 0 -- failure: parent << -1
pid = fork();
if (pid == -1) {
printf("Error executing command\n");
continue;
} else if (pid == 0) {
execute_cmd(file_name, args);
} else {
pid = wait(status);
if (pid == -1) {
printf("%s\n", strerror(errno));
exit(EXIT_FAILURE);
}
}
tokens[0] = "\0";
prompt();
}
free(cmd);
free(tokens);
free(index);
printf("\n"); /* avoids unwanted terminal output for ctrl-D */
}
int main(void) {
signal(SIGINT, signal_handler);
start_shell();
return 0;
}
execvp needs that argument list to be NULL terminated (else it cannot tell the argument count)
You're not adding NULL when you're building your command. So that's undefined behaviour, and an extra ghost args or two are passed to execvp (until it finally finds a NULL or just a good old segfault)
so:
char **tokens = (char **) malloc(sizeof(char *) * num_args); /* command line input parsed into tokens */
should be
char **tokens = malloc(sizeof(char *) * (num_args + 1)); /* command line input parsed into tokens */
and in parse_cmd add the NULL item:
while ((tok = strtok(NULL, delim)) != NULL) {
tokens[*index] = tok;
(*index)++;
} // this is your code, now
tokens[*index] = NULL;
(*index)++; // maybe not necessary
now execvp is passed a NULL terminated string.