## Check the return value of library functions From the [man page](https://man7.org/linux/man-pages/man2/signal.2.html): > signal() returns the previous value of the signal handler. On failure, it returns SIG_ERR, and errno is set to indicate the error. `system()` and `read()` also return something meaningful. Neither of the calls to `signal()` are checked in your application. The calls to `clock_gettime()`, `system()` `read()`, `mkdir()` et cetera also go unchecked. [Note that there is another way of setting the terminal back to cooked mode instead of calling `system()`.](https://stackoverflow.com/a/13129698/20017547) ## Calling async-signal-unsafe functions in signal handlers is undefined behavior `fflush()`, `fprintf()`, `printf()`, `exit()`, and `system()` are not safe to call in a signal handler according to both the ISO C and POSIX standards. See: [signal-safety(7) — Linux manual page](https://man7.org/linux/man-pages/man7/signal-safety.7.html). The behavior is also undefined if the signal handler refers to any object other than `errno` with static storage duration other than by assigning a value to an object declared as `volatile sig_atomic_t`. The variable `paused` is of type `int`. ## `argv[0]` can be `NULL` ```c program_name = argv[0]; ``` This is risky. One can easily pass in `argv[0]` as a null pointer with an `exec()` syscall. Add a check for it, else a subsequent null pointer dereference would invoke undefined behavior. ## Use the `bool` type to denote a binary state ```c case 's': sflag = 1; break; case 'r': rflag = 1; break; ``` Include `stdbool.h` for `bool`, `true`, and `false`. This is not required in C2X, as they are keywords. You could also make a `struct` containing all these flags and pass it around as an argument instead of making all the flags global. I'd also suggest moving this input parsing to a separate function. Furthermore, consider use `EXIT_FAILURE` and `EXIT_SUCCESS` from `stdlib.h` instead of non-standard exit codes. ## In `main()`, `exit(EXIT_SUCCESS)` is equivalent to `return EXIT_SUCCESS` I suggest eliding the calls to `exit()`. ## The call to `strcat()` might write to out-of-bounds memory ```c strncpy(file, getenv("HOME"), 255); file[255] = '\0'; strcat(file, "/.sw"); ``` You failed to verify whether the string returned by `getenv()` was less than 255 characters. The subsequent calls to `strcat()` have the same problem. You're inviting a buffer overflow attack. I suggest using `strncat()`, or using one of the better alternatives to `strcat()/strncat()` that POSIX might provide. The subsequent call to `strncmp()`: ```c else if (strncmp(mode, "w", 1) == 0 ``` can simply be: ```c else if (mode[0] == 'w') ``` You also don't need separate invocations of `fopen()` in each branch. Move the call to the last line of the function and eliminate the unused variable (`return fopen(...)`). ## Prior to C2X, a function with empty parentheses as the argument-list specifies an argument that takes a variadic number of arguments, not zero Specify `void` wherever you are using empty parentheses. I also do not see what the point of `stopwatch.h` is. I'd just elide it. ## Consecutive string literals are concatenated You can replace the 16 calls to `fprintf()` in the help message with: ```c fprintf(out, "\nOptions:\n" " -h, --help Show this help message and exit.\n" " -s, --save Save the final time to ~/.sw/savedtime\n" " -r, --restore Restore time from ~/.sw/savedtime\n" " -x, --exit Exit instead of pausing.\n" " -p, --paused Start in paused state.\n" " -a, --anykey Exit upon any keypress. With -p, will exit upon any keypress after unpausing.\n" "\nControls:\n" " Space Pause or resume the stopwatch.\n" " s Save the current time to ~/.sw/savedtime.\n" " + Add one second to the time.\n" " - Subtract one second from the time.\n" " r Reset the stopwatch to zero.\n" " q Quit.\n"); ```