2

I'm trying to implement I/O redirection in a shell that I'm writing for a Unix-like OS (xV6). In the manual that I'm reading for the OS, I found the following code that would run in the shell for the execution of the cat command:

char *argv[2];
argv[0] = "cat";
argv[1] = 0;
if(fork() == 0) {
    close(0);
    open("input.txt", O_RDONLY);
    exec("cat", argv);
}

I modified the code to run in my shell which has the argv array located in another function, but it maintains the functionality. For some reason, when I run the cat < input.txt the shell outputs:

cat: -: Bad file descriptor
cat: closing standard input: Bad file descriptor

I'm still new to OS programming so I'm not exactly clear on all of the functionality of I/O redirection, but I think the code I have should work. What could be causing the problem. I have the code for the I/O redirection below:

case '<':
    ecmd = (struct execcmd*)cmd;
    rcmd = (struct redircmd*)cmd;

    if(fork() == 0){
      close(0);
      open("input", O_RDONLY);
      execvp(ecmd->argv[0], ecmd->argv );
    }
    runcmd(rcmd->cmd);
    break;

EDIT

I did strace -e open ls and got:

open("/etc/ld.so.cache",    O_RDONLY|O_CLOEXEC) = 3
open("/lib/x86_64-linux-gnu/libselinux.so.1", O_RDONLY|O_CLOEXEC) = 3
open("/lib/x86_64-linux-gnu/libacl.so.1", O_RDONLY|O_CLOEXEC) = 3
open("/lib/x86_64-linux-gnu/libc.so.6", O_RDONLY|O_CLOEXEC) = 3
open("/lib/x86_64-linux-gnu/libpcre.so.3", O_RDONLY|O_CLOEXEC) = 3
open("/lib/x86_64-linux-gnu/libdl.so.2", O_RDONLY|O_CLOEXEC) = 3
open("/lib/x86_64-linux-gnu/libattr.so.1", O_RDONLY|O_CLOEXEC) = 3
open("/proc/filesystems", O_RDONLY)     = 3
open("/usr/lib/locale/locale-archive", O_RDONLY|O_CLOEXEC) = 3
a.out  sh.c  test
+++ exited with 0 +++

EDIT 2

For some reason, this code for the case works, but I'm not sure why:

case '<':
    rcmd = (struct redircmd*)cmd;
    close(rcmd->fd);
    if(open(rcmd->file, rcmd->mode) < 0){
      printf(2, "Cannot open file %s\n", rcmd->file);
      perror(rcmd->file);
      exit(1);
    }
    runcmd(rcmd->cmd);
    break;
loremIpsum1771
  • 2,497
  • 5
  • 40
  • 87
  • 2
    Can you print out the value that `open()` returns in your code? – Mark Plotnick Sep 14 '15 at 18:11
  • @MarkPlotnick How do I check what open() returns? In the shell it only outputs the bad descriptor message. – loremIpsum1771 Sep 14 '15 at 18:14
  • You can also try running your program under `strace` to trace all the system calls it makes: `strace -f -o /some/output/file/name program [args]`. See http://linux.die.net/man/1/strace – Andrew Henle Sep 14 '15 at 18:15
  • You could assign the result of `open` to a variable and use `printf` to print the value of the variable. Look in your book for the `fork` example to see how a variable assignment and a call to `printf` could be done. – Mark Plotnick Sep 14 '15 at 18:25
  • @AndrewHenle I'll edit the post with what the output was – loremIpsum1771 Sep 14 '15 at 18:26
  • @MarkPlotnick Before, I check that, I think I might have needed to do something like : ```close(STDIN);``` and then ```dup(p[STDIN]);``` before I exec the command. I found this from an [SO answer](http://stackoverflow.com/questions/11635219/dup2-dup-why-would-i-need-to-duplicate-a-file-descriptor). It seems to be dealing with piping but the solution should be similar I think. – loremIpsum1771 Sep 14 '15 at 18:39
  • `dup` copies an existing already-open file descriptor to a new file descriptor (it allocates the lowest-numbered one that's available). Is that what you want to do? – Mark Plotnick Sep 14 '15 at 19:00
  • @MarkPlotnick I think so. ```dup``` was used in one of the code snippets in the manual in the section on I/O redirection which is what I'm trying to implement so I think that I need to use it. Page 10-12 [here](http://pdos.csail.mit.edu/6.828/2012/xv6/book-rev7.pdf) is where I'm getting the information/code from. – loremIpsum1771 Sep 14 '15 at 19:20
  • @loremIpsum1771 You need to use `-f` to also follow child processes since it's the `open()` call in the child process that you're interested in. – Andrew Henle Sep 14 '15 at 20:52
  • @AndrewHenle Do you mean when I run a command like cat > input.txt ? – loremIpsum1771 Sep 14 '15 at 21:25
  • @loremIpsum1771 Yes. If you want to trace child processes also - which in this case you do - you need to use one of the `-f` options of `strace`. – Andrew Henle Sep 14 '15 at 23:57
  • @AndrewHenle Just edited the post with solution that I got to work, though I'm not sure why it works. – loremIpsum1771 Sep 15 '15 at 00:29
  • 1
    The code in the book implements a very specific instance of I/O redirection: reading from a file named `input.txt`. If that file does not exist, `open` will return the special value `-1` which is not a valid file descriptor and is used to indicate an error. The second code snippet is better; it appears to use a user-supplied filename, and checks the value returned from `open`. – Mark Plotnick Sep 15 '15 at 10:09

1 Answers1

5

You can't just close stdin and then open another file. It doesn't automatically become your new stdin. What you want is to use the syscall dup2 which can "redirect" a file descriptor to another.

int fd = open("input.txt", O_RDONLY);
dup2(fd, 0); // stdin now points to fd
close(fd);

For more info see man 2 dup2. Note that open and dup2 can both fail, so you should check their return values if this is a concern.

EDIT: This will actually work sometimes because POSIX guarantees the kernel will always allocate the lowest free file-descriptor. But it is not thread-safe and is just bad style in general. I recommend to always do the dup2 even though in some cases you could get away without it.

jforberg
  • 6,537
  • 3
  • 29
  • 47
  • I just edited the post with the code that seems to be working. Do you know why this code works? It doesn't seem to be doing much more than closing the previous fd. It doesn't even need to dup. – loremIpsum1771 Sep 15 '15 at 00:30
  • 1
    When you open a file, the kernel will usually select the next free file descriptor. So the first file you open will be fd 3, then fd 4 etc. When you close fd 0, it becomes free and the kernel will assign it next. However the kernel is not required to assign FDs in any specific order and coding like this is very bad style. Also, if you found my answer helpful please consider "accepting" it. EDIT: apparently the kernel **is** required to always select the lowest available FD. I would still consider this bad style, though. http://pubs.opengroup.org/onlinepubs/009695399/functions/open.html – jforberg Sep 15 '15 at 14:36