Unix 'sleep' clone - unsure of safety

Question

I decided to try making some simple clones of the various user-space Unix utilities in an effort to learn the system-specific headers and improve my C skills. The code that follows is a clone of sleep and I'm curious as to whether there are any pitfalls I'm overlooking (including security vulnerabilities). It compiles and seems to behave fine so there's no issue there.

The program attempts to comply with the Open Group spec for sleep.

#include <ctype.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#define MAX_TIME_DIGITS (11)

int main(int argc, char *argv[]) {
    if(argc != 2) {
        fprintf(stderr, "sleep: too %s operands\n",
                argc < 2 ? "few" : "many");
        return 1;
    }

    if(strnlen(argv[1], MAX_TIME_DIGITS + 1) > MAX_TIME_DIGITS) {
        fprintf(stderr, "sleep: operand too long\n");
        return 1;
    }

    int i = 0;
    while(argv[1][i] && i < MAX_TIME_DIGITS) {
        if(!isdigit((int)argv[1][i])) {
            fprintf(stderr, "sleep: invalid operand\n");
            return 1;
        }
        ++i;
    }

    unsigned long int time = strtoul(argv[1], NULL, 10);
    sleep(time);
    return 0;
}

Answer 1

• An error message ("too few"/"too many") is not helpful. Typically when an Unix utility cannot deal with the command line it prints what it expects:

  For example,

  fprintf(stderr, " Usage: %s sleep_duration", argv[0])
  

  It is also customary to provide a usage() function to print such message.

  In any case, do not hardcode a sleep string. Use argv[0].

• strtoul has very good error handling capabilities. It is pointless to try to intercept problems it can detect.

  Just let it do its job:

  char * stop;
  unsigned long int time = strtoul(argv[1], &stop, 10);
  if (*stop != 0) {
      // strtoul failed to parse the argument. Now printing an error message is appropriate.
      usage("...");
  }
  

• It would be nice to add an option to define a SIGALRM behaviour. OpenGroup does not specify which action shall be taken, meaning it is up to you to define it.