On first glance, other than your close and dup2 being out of order in your toks[in] case, there isn't anything readily apparent that explains why you do not create an output file when redirecting (e.g. cat somefile > newfile). However, there are a number of subtleties that you are not checking.
For example, you need to check whether your call to open succeeds in each case before calling dup2 and close. (otherwise, you are attempting to redirect a file-descriptor that is not open). Simple basic checking will do, e.g.
if (in) {
int fd0;
if ((fd0 = open(toks[in], O_RDONLY)) == -1) {
perror (toks[in]);
exit (EXIT_FAILURE);
}
dup2(fd0, 0);
close(fd0);
}
if (out)
{
int fd1;
if ((fd1 = open(toks[out],
O_WRONLY | O_CREAT | O_TRUNC | O_CREAT,
S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH)) == -1) {
perror (toks[out]);
exit (EXIT_FAILURE);
}
dup2(fd1, 1);
close(fd1);
}
(note: I've tweaked the permission to write the file as 0644 (user 'rw', group 'r' and world 'r'. You should also check the returns of dup2 and in the pedantic case close)
The bigger issues come in how you rearrange toks before your call to execvp. The reason you use dup2 or a pipe is that the exec.. function cannot handle redirection (e.g. it doesn't know what to do with '>' or '<'). So you are manually handling the redirection of input or output to a file by redirecting either the file to stdin on the input case or stdout (and/or stderr) to the file on the output case. In either case, you must remove the < filename or > filename tokens from toks before calling execvp or you will get an error.
If you insure that set each pointer in toks to NULL and you read no more than MAXTOKS - 1 (preserving a terminating NULL as required), then you can iterate over toks shifting the pointers to insure you do not send the < > and filename to execvp. After you find < or > in toks at index i and insure there is a toks[i+1] filename, something like:
while (toks[i]) {
toks[idx] = toks[i+2];
i++;
}
Then passing toks to execvp will not generate the error (that I suspect is what you are experiencing)
There is also another corner-case nit you should be aware of. If your executable has any registered calls to atexit or other desctructors, the references are not part of your call to execvp. So if the call to execvp fails, you cannot call exit (which can invoke undefined behavior in a call to any post-exit function), so the proper call is to _exit which will not attempt any such calls.
A bare minimum of the working redirection would be something like the following. Not there are many other aspects of parsing and redirection not addressed below, but for your basic file creation problem, it provides a framework, e.g.
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <errno.h>
#include <fcntl.h>
#include <signal.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <unistd.h>
enum {ARGSIZE = 20, BUF_SIZE = 1024}; /* constants */
void execute (char **args);
int main (void) {
while (1) {
char line[BUF_SIZE] = "",
*args[ARGSIZE],
*delim = " \n",
*token;
int argIndex = 0;
for (int i = 0; i < ARGSIZE; i++) /* set all pointers NULL */
args[i] = NULL;
printf ("shell> "); /* prompt */
if (!fgets (line, BUF_SIZE, stdin)) {
fprintf (stderr, "Input canceled - EOF received.\n");
return 0;
}
if (*line == '\n') /* Enter alone - empty line */
continue;
for (token = strtok (line, delim); /* parse tokens */
token && argIndex + 1 < ARGSIZE;
token = strtok (NULL, delim)) {
args[argIndex++] = token;
}
if (!argIndex) continue; /* validate at least 1 arg */
if (strcmp (args[0], "quit") == 0 || strcmp (args[0], "exit") == 0)
break;
execute (args); /* call function to fork / execvp */
}
return 0;
}
void execute (char **args)
{
pid_t pid, status;
pid = fork ();
if (pid < 0) {
perror ("fork");
return;
}
else if (pid > 0) {
while (wait (&status) != pid)
continue;
}
else if (pid == 0) {
int idx = 0,
fd;
while (args[idx]) { /* parse args for '<' or '>' and filename */
if (*args[idx] == '>' && args[idx+1]) {
if ((fd = open (args[idx+1],
O_WRONLY | O_CREAT,
S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH)) == -1) {
perror (args[idx+1]);
exit (EXIT_FAILURE);
}
dup2 (fd, 1);
dup2 (fd, 2);
close (fd);
while (args[idx]) {
args[idx] = args[idx+2];
idx++;
}
break;
}
else if (*args[idx] == '<' && args[idx+1]) {
if ((fd = open (args[idx+1], O_RDONLY)) == -1) {
perror (args[idx+1]);
exit (EXIT_FAILURE);
}
dup2 (fd, 0);
close (fd);
while (args[idx]) {
args[idx] = args[idx+2];
idx++;
}
break;
}
idx++;
}
if (execvp (args[0], args) == -1) {
perror ("execvp");
}
_exit (EXIT_FAILURE); /* must _exit after execvp return, otherwise */
} /* any atext calls invoke undefine behavior */
}
Example Use/Output
Minimally working both > filename and < filename,
$ ./bin/execvp_wredirect
shell> ls -al tmp.txt
ls: cannot access 'tmp.txt': No such file or directory
shell> cat dog.txt
my dog has fleas
shell> cat dog.txt > tmp.txt
shell> ls -al tmp.txt
-rw-r--r-- 1 david david 17 Feb 25 01:52 tmp.txt
shell> cat < tmp.txt
my dog has fleas
shell> quit
Let me know if this solves the error issue. The only other creation issue would be you don't have write permission where you are attempting to create the file. If this doesn't solve the issue, please post all your code in a MCVE so I can insure that problems are not created in other areas of the code.
After Post of Your Complete Code
Your biggest issue was in your use of strtok_r and not removing the filename (or setting it NULL before calling execvp), and in using i + 1 instead of i in your assignment to in and out, e.g.
tok = strtok_r(rest, delim, &rest);
while(tok != NULL && i < MAX_TOKS)
{
toks[i] = tok;
if(strcmp(tok, "<") == 0)
{
in = i;
i--;
}
else if(strcmp(tok, ">")==0)
{
out = i;
i--;
}
i++;
tok = strtok_r(NULL, delim, &rest);
}
When you used i + 1, you set the index for tok[in] or tok[out] to one past the filename prompting the Bad Address error. It's one of those Doah! (or "id10t") errors... (rewrite the quote all-caps)
Further, before your call to execvp you must set tok[in] or tok[out] to NULL as you have removed the < and > and the file descriptor has already been duped, e.g.
dup2(fd0, 0);
close(fd0);
toks[in] = NULL;
and
dup2(fd1, 1);
close(fd1);
toks[out] = NULL;
You had also forgotten to reset your loop variables, e.g.
while (1)
{
in = out = 0; /* always reset loop variables */
for (int i = 0; i < MAX_TOKS; i++)
toks[i] = NULL; /* and NULL all pointers */
Cleaning what you had up a bit, you could do something like the following:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
#include <unistd.h>
#include <errno.h>
#include <fcntl.h>
#include <sys/types.h> /* missing headers */
#include <sys/wait.h>
#define MAX_BUF 160
#define MAX_TOKS 100
int main(int argc, char **argv)
{
char *delim = " \n"; /* delimiters for strtok_r (including \n) */
// char *pos; /* no longer used */
char *tok;
char *path;
char s[MAX_BUF];
char *toks[MAX_TOKS];
time_t rawtime;
struct tm *timeinfo;
static const char prompt[] = "msh> ";
FILE *infile;
int in;
int out;
// int fd0; /* unused and shadowed declarations below */
// int fd1; /* always compile with -Wshadow */
in = 0;
out = 0;
/*
* process command line options
*/
if (argc > 2) {
fprintf(stderr, "msh: usage: msh [file]\n");
exit(EXIT_FAILURE);
}
if (argc == 2) {
/* read from script supplied on the command line */
infile = fopen(argv[1], "r");
if (infile == NULL) {
fprintf(stderr, "msh: cannot open script '%s'.\n", argv[1]);
exit(EXIT_FAILURE);
}
} else {
infile = stdin;
}
while (1)
{
in = out = 0; /* always reset loop variables */
for (int i = 0; i < MAX_TOKS; i++)
toks[i] = NULL;
// prompt for input, if interactive input
if (infile == stdin) {
printf(prompt);
}
/*
* read a line of input and break it into tokens
*/
// read input
char *status = fgets(s, MAX_BUF-1, infile);
// exit if ^d or "exit" entered
if (status == NULL || strcmp(s, "exit\n") == 0) {
if (status == NULL && infile == stdin) {
printf("\n");
}
exit(EXIT_SUCCESS);
}
// break input line into tokens
char *rest = s;
int i = 0;
tok = strtok_r(rest, delim, &rest);
while(tok != NULL && i < MAX_TOKS)
{
toks[i] = tok;
if(strcmp(tok, "<") == 0)
{
in = i; /* only i, not i + 1, you follow with i-- */
i--;
}
else if(strcmp(tok, ">")==0)
{
out = i; /* only i, not i + 1, you follow with i-- */
i--;
}
i++;
tok = strtok_r(NULL, delim, &rest);
}
if (i == MAX_TOKS) {
fprintf(stderr, "msh: too many tokens");
exit(EXIT_FAILURE);
}
toks[i] = NULL;
/*
* process a command
*/
// do nothing if no tokens found in input
if (i == 0) {
continue;
}
// if a shell built-in command, then run it
if (strcmp(toks[0], "help") == 0) {
// help
printf("enter a Linux command, or 'exit' to quit\n");
continue;
}
if (strcmp(toks[0], "today") == 0) {
// today
time(&rawtime);
timeinfo = localtime(&rawtime);
printf("Current local time: %s", asctime(timeinfo));
continue;
}
if (strcmp(toks[0], "cd") == 0)
{
// cd
if (i == 1) {
path = getenv("HOME");
} else {
path = toks[1];
}
int cd_status = chdir(path);
if (cd_status != 0)
{
switch(cd_status)
{
case ENOENT:
printf("msh: cd: '%s' does not exist\n", path);
break;
case ENOTDIR:
printf("msh: cd: '%s' not a directory\n", path);
break;
default:
printf("msh: cd: bad path\n");
}
}
continue;
}
// not a built-in, so fork a process that will run the command
pid_t rc = fork(), rcstatus; /* use type pid_t, not int */
if (rc < 0)
{
fprintf(stderr, "msh: fork failed\n");
exit(1);
}
if (rc == 0)
{
if(in)
{
int fd0;
if((fd0 = open(toks[in], O_RDONLY, 0)) == -1)
{
perror(toks[in]);
exit(EXIT_FAILURE);
}
dup2(fd0, 0);
close(fd0);
toks[in] = NULL;
}
if(out)
{
int fd1;
if((fd1 = open(toks[out], O_WRONLY | O_CREAT | O_TRUNC | O_CREAT,
S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH)) == -1)
{
perror (toks[out]);
exit( EXIT_FAILURE);
}
dup2(fd1, 1);
close(fd1);
toks[out] = NULL;
}
// child process: run the command indicated by toks[0]
execvp(toks[0], toks);
/* if execvp returns than an error occurred */
printf("msh: %s: %s\n", toks[0], strerror(errno));
exit(1);
}
else
{
// parent process: wait for child to terminate
while (wait (&rcstatus) != rc)
continue;
}
}
}
You will need to verify there are no additional issues, but it certainly has no problems with cat file1 > file2.
