This is a simple stopwatch cli application with a few options

Question

My goal of this when I started writing it was simply that it be able to record thousandths of a second in a very performant and accurate way. I'm not sure I have achieved the performance aspect of that though, and I am not sure how to check the accuracy. I haven't done a lot of C programming before, but I feel like there is a lot wrong with this code. One thing I noticed is that my CPU usage spikes if I leave this running in the VSCode terminal with it open, but drops down when the terminal is hidden. Maybe I need to print to the screen less often. And I have successfully built this on both macos and Linux, but I know little about portability. Anyway, the code.

Here is my header:

#ifndef STOPWATCH_H
#define STOPWATCH_H

#include <stdio.h>

void set_canonical_mode(int enable);
void print_time(FILE *fd);
void clear_output();
FILE *get_saved_time_file(char *mode);
void save_time();
void restore_time();
void add_one_second();
void subtract_one_second();
void reset_time();
void cleanup();
void sigint_handler(int sig);
void* input_thread();
void print_help(FILE *out);
void print_short_help(FILE *out);

#endif

And here is the main file:

#include "sw.h"
#include <getopt.h>
#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/stat.h>
#include <termios.h>
#include <time.h>
#include <unistd.h>

#define ONE_MILLION_NSEC 1000000
#define ONE_BILLION_NSEC 1000000000
#define NSEC_TO_SLEEP ONE_MILLION_NSEC
#define HIDE_CURSOR printf("\e[?25l")
#define SHOW_CURSOR printf("\e[?25h")

#define OVER_HOUR_TEMPLATE "%2d:%02d:%02d.%02d"
#define UNDER_HOUR_TEMPLATE "%02d:%02d.%02d"

int paused = 0;

const struct timespec sleepduration = {0, NSEC_TO_SLEEP};

struct timespec starttime;
struct timespec currenttime;
struct timespec elapsedtime;
struct timespec restored_time;
struct timespec resumedtime;

char endchar = '\r';

int rflag = 0;
int sflag = 0;
int xflag = 0;
int pflag = 0;
int aflag = 0;

static char *program_name;

void set_raw_mode(int enable) {
  struct termios term_settings;
  tcgetattr(STDIN_FILENO, &term_settings);

  if (enable) {
    term_settings.c_lflag &= ~(ICANON | ECHO);
  } else {
    system("stty sane");
  }

  tcsetattr(STDIN_FILENO, TCSANOW, &term_settings);
}

void print_time(FILE *fd) {
  if (elapsedtime.tv_sec < 3600) {
    fprintf(fd, UNDER_HOUR_TEMPLATE, (int)(elapsedtime.tv_sec / 60),
            (int)(elapsedtime.tv_sec % 60),
            (int)(elapsedtime.tv_nsec / 10000000));
  } else {
    fprintf(fd, OVER_HOUR_TEMPLATE, (int)(elapsedtime.tv_sec / 3600),
            (int)(elapsedtime.tv_sec % 3600 / 60),
            (int)(elapsedtime.tv_sec % 60),
            (int)(elapsedtime.tv_nsec / 10000000));
  }
  fprintf(fd, "%c", endchar);
  fflush(stdout);
}

void pause_timer() {
  paused = 1;
  clock_gettime(CLOCK_MONOTONIC, &currenttime);
  if (xflag) {
    cleanup();
    exit(0);
  }
}

void resume_timer() {
  paused = 0;
  clock_gettime(CLOCK_MONOTONIC, &resumedtime);
  starttime.tv_sec += resumedtime.tv_sec - currenttime.tv_sec;
  starttime.tv_nsec += resumedtime.tv_nsec - currenttime.tv_nsec;
  if (starttime.tv_nsec < 0) {
    starttime.tv_nsec += ONE_BILLION_NSEC;
    --starttime.tv_sec;
  } else if (starttime.tv_nsec >= ONE_BILLION_NSEC) {
    starttime.tv_nsec -= ONE_BILLION_NSEC;
    ++starttime.tv_sec;
  }
}

void clear_output() {
  printf("\r");
  fflush(stdout);
}

FILE *get_saved_time_file(char *mode) {
  FILE *savedtimef = NULL;
  char file[256];
  char *homedirectory = getenv("HOME");
  if (homedirectory != NULL) {
    strncpy(file, getenv("HOME"), 255);
    file[255] = '\0';
    strcat(file, "/.sw");
    struct stat sb;
    if (stat(file, &sb) == 0 && S_ISDIR(sb.st_mode)) {
      strcat(file, "/savedtime");
      savedtimef = fopen(file, mode);
    } else if (strncmp(mode, "w", 1) == 0) {
      mkdir(file, S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH);
      strcat(file, "/savedtime");
      savedtimef = fopen(file, mode);
    }
  }
  return savedtimef;
}

void save_time() {
  FILE *savedtimef = get_saved_time_file("w");
  if (savedtimef == NULL) {
    perror("Could not save to file");
    exit(1);
  }
  print_time(savedtimef);
}

void restore_time() {
  int hours = 0;
  int minutes = 0;
  int seconds = 0;
  int centiseconds = 0;

  FILE *savedtimef = get_saved_time_file("r");

  if (savedtimef == NULL) {
    restored_time.tv_sec = 0;
    restored_time.tv_nsec = 0;
    return;
  }

  int bufsize = 32;
  char *buf = malloc(bufsize * sizeof(char));
  if (buf == NULL) {
    perror("Could not allocate memory to restore previous time.\n");
  }
  memset(buf, 0, bufsize);

  fgets(buf, bufsize, savedtimef);

  if (strlen(buf) < 9) {
    perror("Restore file unparsable.");
  } else if (strlen(buf) == 9) {
    sscanf(buf, UNDER_HOUR_TEMPLATE, &minutes, &seconds, &centiseconds);
    restored_time.tv_sec = (minutes * 60) + seconds;
    restored_time.tv_nsec = (centiseconds * 10000000);
  } else {
    sscanf(buf, OVER_HOUR_TEMPLATE, &hours, &minutes, &seconds, &centiseconds);
    restored_time.tv_sec = (hours * 60 * 60) + (minutes * 60) + seconds;
    restored_time.tv_nsec = (centiseconds * 10000000);
  }
  free(buf);

  starttime.tv_sec -= restored_time.tv_sec;
  starttime.tv_nsec -= restored_time.tv_nsec;
  if (starttime.tv_nsec < 0) {
    starttime.tv_nsec += ONE_BILLION_NSEC;
    --starttime.tv_sec;
  }

  clock_gettime(CLOCK_MONOTONIC, &currenttime);
  elapsedtime.tv_sec = currenttime.tv_sec - starttime.tv_sec;
  elapsedtime.tv_nsec = currenttime.tv_nsec - starttime.tv_nsec;
  if (elapsedtime.tv_nsec < 0) {
    elapsedtime.tv_nsec += ONE_BILLION_NSEC;
    --elapsedtime.tv_sec;
  }
}

void add_one_second() {
  starttime.tv_sec -= 1;
  if (paused) {
    elapsedtime.tv_sec = currenttime.tv_sec - starttime.tv_sec;
    elapsedtime.tv_nsec = currenttime.tv_nsec - starttime.tv_nsec;
    if (elapsedtime.tv_nsec < 0) {
      elapsedtime.tv_nsec += ONE_BILLION_NSEC;
      --elapsedtime.tv_sec;
    }
    print_time(stdout);
  }
}

void subtract_one_second() {
  starttime.tv_sec += 1;
  if (starttime.tv_sec > currenttime.tv_sec ||
      (starttime.tv_sec == currenttime.tv_sec &&
       starttime.tv_nsec > currenttime.tv_nsec)) {
    starttime.tv_sec = currenttime.tv_sec;
    starttime.tv_nsec = currenttime.tv_nsec;
  }
  if (paused) {
    elapsedtime.tv_sec = currenttime.tv_sec - starttime.tv_sec;
    elapsedtime.tv_nsec = currenttime.tv_nsec - starttime.tv_nsec;
    if (elapsedtime.tv_nsec < 0) {
      elapsedtime.tv_nsec += ONE_BILLION_NSEC;
      --elapsedtime.tv_sec;
    }
    print_time(stdout);
  }
}

void reset_time() {
  clock_gettime(CLOCK_MONOTONIC, &starttime);
  if (paused) {
    clock_gettime(CLOCK_MONOTONIC, &currenttime);
    elapsedtime.tv_sec = 0;
    elapsedtime.tv_nsec = 0;
    print_time(stdout);
  }
}

void cleanup() {
  paused = 1;
  endchar = '\n';
  clear_output();
  print_time(stdout);
  SHOW_CURSOR;
  set_raw_mode(0);
  if (sflag) {
    save_time();
  }
}

void sigint_handler(int sig) {
  cleanup();
  exit(sig);
}

void get_input() {
  fd_set read_fds;
  struct timeval timeout;
  FD_ZERO(&read_fds);
  FD_SET(STDIN_FILENO, &read_fds);
  timeout.tv_sec = 0;
  timeout.tv_usec = 1000; // Check every 1ms
  if (select(STDIN_FILENO + 1, &read_fds, NULL, NULL, &timeout) == 1) {
    char c;
    read(STDIN_FILENO, &c, 1);
    if (aflag) {
      if (pflag) {
        if (c == ' ') {
          pflag = !pflag;
        }
      } else {
        // exit upon any keypress
        cleanup();
        exit(0);
      }
    }
    switch (c) {
    case ' ':
      // pause or resume stopwatch
      if (paused)
        resume_timer();
      else
        pause_timer();
      break;
    case 's':
      save_time();
      break;
    case 'r':
      reset_time();
      break;
    case '+':
      add_one_second();
      break;
    case '-':
      subtract_one_second();
      break;
    case 'q':
      // quit
      cleanup();
      exit(0);
      break;
    default:
      break;
    }
  }
}

void print_short_help(FILE *out) {
  fprintf(out, "Usage: %s [-hsrxpa]\n", program_name);
}

void print_help(FILE *out) {
  print_short_help(stdout);
  fprintf(out, "\nOptions:\n");
  fprintf(out, "  -h, --help    Show this help message and exit.\n");
  fprintf(out, "  -s, --save    Save the final time to ~/.sw/savedtime\n");
  fprintf(out, "  -r, --restore Restore time from ~/.sw/savedtime\n");
  fprintf(out, "  -x, --exit    Exit instead of pausing.\n");
  fprintf(out, "  -p, --paused  Start in paused state.\n");
  fprintf(out, "  -a, --anykey  Exit upon any keypress. With -p, will exit "
               "upon any keypress after unpausing.\n");

  fprintf(out, "\nControls:\n");
  fprintf(out, "  Space         Pause or resume the stopwatch.\n");
  fprintf(out, "  s             Save the current time to ~/.sw/savedtime.\n");
  fprintf(out, "  +             Add one second to the time.\n");
  fprintf(out, "  -             Subtract one second from the time.\n");
  fprintf(out, "  r             Reset the stopwatch to zero.\n");
  fprintf(out, "  q             Quit.\n");
}

int main(int argc, char *argv[]) {
  signal(SIGINT, sigint_handler);
  signal(SIGTERM, sigint_handler);

  program_name = argv[0];

  static struct option long_options[] = {
      {"help", no_argument, 0, 'h'},    {"save", no_argument, 0, 's'},
      {"restore", no_argument, 0, 'r'}, {"exit", no_argument, 0, 'x'},
      {"paused", no_argument, 0, 'p'},  {"anykey", no_argument, 0, 'a'},
  };

  int opt;
  int option_index = 0;
  while ((opt = getopt_long(argc, argv, "hsrxpa", long_options,
                            &option_index)) != -1) {
    switch (opt) {
    case 'h':
      print_help(stdout);
      exit(0);
    case 's':
      sflag = 1;
      break;
    case 'r':
      rflag = 1;
      break;
    case 'x':
      xflag = 1;
      break;
    case 'p':
      pflag = 1;
      break;
    case 'a':
      aflag = 1;
      break;
    case '?':
      print_short_help(stderr);
      break;
    default:
      exit(1);
    }
  }

  set_raw_mode(1);
  HIDE_CURSOR;

  clock_gettime(CLOCK_MONOTONIC, &starttime);
  if (rflag) {
    restore_time();
  }
  if (pflag) {
    pause_timer();
    print_time(stdout);
    while (paused) {
      get_input();
      nanosleep(&sleepduration, NULL);
    }
  }

  while (1) {
    if (!paused) {
      clock_gettime(CLOCK_MONOTONIC, &currenttime);
      elapsedtime.tv_sec = currenttime.tv_sec - starttime.tv_sec;
      elapsedtime.tv_nsec = currenttime.tv_nsec - starttime.tv_nsec;
      if (elapsedtime.tv_nsec < 0) {
        elapsedtime.tv_nsec += ONE_BILLION_NSEC;
        --elapsedtime.tv_sec;
      }
      print_time(stdout);
    }
    get_input();
    nanosleep(&sleepduration, NULL);
  }
  return 0;
}

Here are also some future features I'd like to add. Would welcome advice on implementation:

• option to specify timer precision
• option to specify time formatting (e.g., "01:23" vs "1m23s" vs "1 minute 23 seconds".)
• option to give filepath to save to/restore from (should this be done with pipes/redirection?)
• option to start timer on keyUP. If you've ever used a rubik's cube mat, they being timing when you hand leaves the mat.
• option to set laps, and have them be saved/restored from file as well.

Answer 1

Check the return value of library functions

From the man page:

signal() returns the previous value of the signal handler. On failure, it returns SIG_ERR, and errno is set to indicate the error.

Neither of the calls to signal() are checked in your application. The calls to clock_gettime(), system() read(), mkdir(), and fgets() et cetera also go unchecked.

Note that there is another way of setting the terminal back to cooked mode instead of calling system().

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.

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

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

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.

#define C2X_PLACEHOLDER 202000L

#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= C2X_PLACEHOLDER
    /* Coast is clear. */
#else
    #include <stdbool.h>            /* bool, true, and false. */
#endif

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 using 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

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():

else if (strncmp(mode, "w", 1) == 0

can simply be:

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:

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");

The identifier fd is conventionally used for a file descriptor, and not a FILE *

Consider using fp or stream. Additionally, the arguments that are not modified in the function should be const-qualified. If you're using C99 or higher, you should also be using the ptr[static 1] notation for pointers that are expected to be non-null.

Do not use dynamic allocation when a fixed-size array would suffice

In here:

int bufsize = 32;
char *buf = malloc(bufsize * sizeof(char));
if (buf == NULL) {
  perror("Could not allocate memory to restore previous time.\n");
}
memset(buf, 0, bufsize);

The size of the buffer is already known, and there's no reason to call malloc() for it. Simply do:

#define BUFSIZE 32
char buf[BUFSIZE];

To zero-initialize it, which I don't believe has any value here:

char buf[BUFSIZE] = {0};

// Or in C2X, like C++:
char buf[BUFSIZE] = {};

And malloc()+memset() should really be calloc().

Also note that sizeof(char) is defined by the standard to be 1. So it can be safely elided.

Additionally, use an extra variable to avoid the two calls to strlen().

Answer 2

This looks silly at first:

#define ONE_MILLION_NSEC 1000000
#define ONE_BILLION_NSEC 1000000000

But considering that you cannot yet write 1'000'000 in C, this will make your code easier to read. However, you don’t use these macros!

void print_time(FILE *fd) {
  if (elapsedtime.tv_sec < 3600) {
    fprintf(fd, UNDER_HOUR_TEMPLATE, (int)(elapsedtime.tv_sec / 60),
            (int)(elapsedtime.tv_sec % 60),
            (int)(elapsedtime.tv_nsec / 10000000));
  } …

I need to count zeros, which is not easy. I think you can replace that last line with

            (int)(elapsedtime.tv_nsec / (10 * ONE_MILLION_NSEC)));

I prefer to use E notation (1e6), but I understand when people want to keep it all as integers.

That bit of code I copied above has to other constants. I suggest you add those as macros also:

#define SECS_PER_MIN 60
#define SECS_PER_HOUR 3600

---

The other thing I wanted to comment on is program structure. You have lots of global variables that don’t need to be global — ideally you’d have zero globals. Also you have a single .c file that defines main and all the functions it calls, so what is the .h file useful for? You cannot include this code in another project, because it defines main. So this header can only ever be included in this .c file here. But it doesn’t declare anything that the .c file doesn’t already know.

I would suggest writing either a single-file program, or split up the code into meaningful, reusable modules (maybe one .c and .h that define a reusable stopwatch mechanism, and one .c file that contains main and does the user interaction).

---

You ask about portability. You’re using POSIX functions, so this will not compile on Windows.