CSAPP Shell Lab

Repo: ovfor4/tiny-shell

An advanced version ovfor4/ccshell

Not a good idea to paste every line of code on the website if you have a github account

Some useful notes here, not cheating:

Intro

So relatively speaking it’s a simple lab, and I only spent 3 days (6 hrs in total), but signals are tricky

And I’m gonna go beyond the syllabus, code more stuff and optimize user experience

And I used C++ instead of C, but they are fairly similar, and the key of this lab is POSIX stuff, so just ignore this point

By the way, I plan to rewrite the whole stuff in C++ to reduce bad habit (eg **argv if you aren’t communicating with the system), and maybe add some features?

Utils

Easy, wrap your own write so that it can output both numbers and strings without typing strlen and STDOUT_FILENO

Handlers

At the beginning of the handler, backup errno and block other signals that can race, in this case, SIGINT SIGCHLD SIGTSTP

SIG INT/TSTP handler

Find the foreground process and send SIGINT/SIGTSTP to the whole group

SIG CHLD handler

Signals aren’t queued, and when the program receives a signal, perhaps there are many children terminated/suspended, so while is used to process all of these

And in this lab, the job status is changed for termination and suspension. WNOHANG means return immediately if there aren’t any more children to process, and WUNTRACED means consider SIGTSTP

If return value is >0, there is at least 1 child

And this expression can be obtained

((pid = waitpid(-1, &status, WNOHANG | WUNTRACED)) > 0)

And then we need to determine the reason, and print corresponding message. Just check the code if interested

These might be useful:

• WIFSTOPPED(status)
• WTERMSIG(status)
• WSTOPSIG(status)
• WTERMSIG(status)

waitfg

This one is VERY tricky

SIGCHLD MUST be blocked before child is forked and CANNOT be unblocked manually

Before waiting for SIGCHLD, we need to unblock other signals. Otherwise, for example, SIGINT is blocked and thus the child cannot receive, and thus cannot terminate, but waitfg is still waiting for SIGCHLD, which can never happen

int sigsuspend(const sigset_t *__set) changes the block set to set, waits for signal, and restores to the previous set before this instruction runs. And it’s noticeable that it’s atomic, so it’s impossible that SIGCHLD is unblocked and received before we are ready to catch this change (that’s why we can’t unblock manually, otherwise there can be a gap)

do_bgfg

First, do some validation to make sure the input is legal

And get both job pointer and pid, so we can do something easily afterward

fg

1. change status to FG
2. block SIGCHLD and send SIGCONT (sig continue) to the process GROUP
3. waitfg

bg

Same, but without waitfg

eval

int parseline(const char *cmdline, char **argv) requires argv[][], we need to convert cmdline[] to argv[][] format first, and check whether it’s foreground or background command

And test whether it’s built-in command or not

And block all signals to prepare to fork child

Remember, fork() returns twice, 0 to the child, and the pid of child to the parent

Inside the child, signals must be unblocked otherwise it will be blocked even after execve (bad, the program inside doesn’t know all signals are blocked LOL)

Inside the parent, it does different actions based on whether it’s foreground or background, and this is similar to do_bgfg. just check that chapter

And that’s it! It’s working

Issues in “official” solution

It’s official perhaps, because tshref.c was decompiled from the binary and I hope it’s correct (you just ask claude or what)

But there are some problems

• [212] sigprocmask(SIG_UNBLOCK, &mask, NULL); is before [217] waitfg(pid); so how can you make sure SIGCHLD is not received in this gap? The “solution” is sleep(1) and access job[] every time! (sigsuspend is better, because it atomically switch to the set and wait for signals, preventing any gaps)

• And errno isn’t saved in another variable and restored before returing, and it’s a good habit to always do so

• And printf are used everywhere even inside signal handler, and we know signal handler should use async-safe functions (man 7 signal-safety), so write should be used instead (that’s why some people feel really uncomfortable when doing signal handlers, because you want the main thread to check something and output, and really, outputing in handler isn’t good anyway, just simple, because write itself is simple and ignores buffer and lock and other meaningful stuff) 👉 TIP: if you are using C++, ... arguments can be implemented easily

• And [23] #define MAXJID 1<<16 is not safe (macro trick). Consider cout << MAXJID;? It would be cout << 1 << 16; (ie 116). And this MAXJID only appears once in the file, perhaps mistake

• And autograder doesn’t work (see my environment later, but anyway it’s 2003 work)

• And even more? Perhaps