Merge branch 'release/1.3'
[reservoir_sample] / reservoir_sample.c
index 1c64f6fe5598179395241f48c643df05c4b94a45..50a1235642f9d2528fa7db8da096ec829486339f 100644 (file)
@@ -51,8 +51,9 @@ static struct options_ {
        .status_fd = STDERR_FILENO,
 };
 
-/*     #status_requested_ will be set whenever a view of the current sample
-       reservoir is desired.
+/*     #status_requested_
+               This will be set whenever a snapshot of the current state of the sample
+       reservoir is desired.  Typically this will occur on a signal.
 */
 static int status_requested_ = 0;
 
@@ -76,9 +77,9 @@ void usage_(const char *prog, unsigned int full) {
                           "\t-n <num>   -- returns <num> samples [default: %zu]\n"
                           "\t-d <delim> -- use <delim> as input and output delimiter [default: '\\%03hho']\n"
                           "\t-r <file>  -- read randomness from <file> [default: '%s']\n"
-                          "\t-b <bytes> -- set input buffer size [default: %zu]\n"
+                          "\t-b <bytes> -- input buffer size [default: %zu]\n"
                           "\t-i <num>   -- grow reservoir by 1 for every <num> input samples (0 inhibits behavior) [default: %lu]\n"
-                          "\t-h <num>   -- double the reservoir growth interval every <num> input samples (0 inhibits behavior) [default: %lu]\n "
+                          "\t-j <num>   -- double the reservoir growth interval every <num> input samples (0 inhibits behavior) [default: %lu]\n "
                           "\t-s <file>  -- USR1 signals will write reservoir contents to <file> rather than stderr (has no effect on normal output to stdout upon input EOF)\n"
                           "\t-v         -- increase verbosity\n"
                           "\t-h         -- this screen\n",
@@ -105,10 +106,13 @@ void request_snapshot_(int signum) {
        status_requested_ = 1;
 }
 
-/*  accumulate_input_
-        Read (up to #read_block_sz bytes at a time) from #fd (until EOF) into an
-    accumulator buffer.  For each #delimiter character found in what was just
-    read, occasionally remember the preceeding characters.
+/*     accumulate_input_
+               Read (up to #read_block_sz bytes at a time, or until EOF) from #fd into
+       an accumulator buffer.  For each #delimiter character found in what was
+       just read, occasionally remember the group of preceeding characters.
+               This behavioral logic lives here, rather than being contained in the
+       reservoir module, simply because it was easier to reduce the number of
+       allocations by handling it here.
 */
 static
 int accumulate_input_(int fd, size_t read_block_sz, int delimiter, unsigned long *psamples, reservoir_t *preservoir) {
@@ -137,10 +141,15 @@ int accumulate_input_(int fd, size_t read_block_sz, int delimiter, unsigned long
        for (;;) {
                NOTIFY_DEBUG("read loop\n\n");
 
-               /* make sure there's enough room in our input buffer for a full read() */
+               /*
+                               Make sure there's enough room in our input buffer to append a
+                       full read() of #read_block_sz onto the end.
+               */
                if (buf_makeroom(&read_buf, read_block_sz)) {
-                       free(read_buf);
-                       free(new_buf);
+                       int e = errno;
+                       buf_del(&read_buf);
+                       buf_del(&new_buf);
+                       errno = e;
                        return -1;
                }
 
@@ -148,8 +157,12 @@ int accumulate_input_(int fd, size_t read_block_sz, int delimiter, unsigned long
                do {
                        len = read(fd, read_buf->buf + read_buf->buf_start + read_buf->buf_used, BUF_ROOM(read_buf));
 
-                       /* a signal may have interrupted read(), deal with that before
-                          doing anything else */
+                       /*
+                                       A signal may have interrupted read(), deal with that before
+                               doing anything else.  Status output is best handled here anyhow,
+                               due to the naturally quiescent state of things, and also due to
+                               not being about to finish and output the final state.
+                       */
                        if (status_requested_) {
                                status_requested_ = 0;
                                NOTIFY_DEBUG("dumping reservoir due to signal");
@@ -163,8 +176,8 @@ int accumulate_input_(int fd, size_t read_block_sz, int delimiter, unsigned long
                } while (len == -1 && (errno == EINTR || errno == EAGAIN));
                if (len < 0) {
                        NOTIFY_ERROR("%s:%s", "read", strerror(errno));
-                       free(read_buf);
-                       free(new_buf);
+                       buf_del(&read_buf);
+                       buf_del(&new_buf);
                        return -1;
                }
                if (len == 0) {
@@ -175,6 +188,10 @@ int accumulate_input_(int fd, size_t read_block_sz, int delimiter, unsigned long
                NOTIFY_DEBUG("len:%zd", len);
                D_BUF("read_buf: ", read_buf);
 
+               /*
+                       Accumulator has been filled, pare and process.
+               */
+
                while (len > 0) {
                        ssize_t len_flensed;
 
@@ -186,7 +203,7 @@ int accumulate_input_(int fd, size_t read_block_sz, int delimiter, unsigned long
                                grow_count = 0;
                                if (reservoir_grow(preservoir, 1)) {
                                        NOTIFY_ERROR("failed to increase reservoir size");
-                                       free(read_buf);
+                                       buf_del(&read_buf);
                                        return -1;
                                }
                        }
@@ -221,7 +238,7 @@ int accumulate_input_(int fd, size_t read_block_sz, int delimiter, unsigned long
                                        NOTIFY_DEBUG("next buffer will be remembered..");
                                        new_buf = buf_new(0);
                                        if (new_buf == NULL) {
-                                               free(read_buf);
+                                               buf_del(&read_buf);
                                                return -1;
                                        }
                                } else {
@@ -231,8 +248,8 @@ int accumulate_input_(int fd, size_t read_block_sz, int delimiter, unsigned long
 
                        len_flensed = buf_flense(&read_buf, bytes_scanned, delimiter, new_buf ? &new_buf : NULL);
                        if (len_flensed < 0) {
-                               free(read_buf);
-                               free(new_buf);
+                               buf_del(&read_buf);
+                               buf_del(&new_buf);
                                return -1;
                        }
                        if (len_flensed == 0) {
@@ -274,18 +291,36 @@ int accumulate_input_(int fd, size_t read_block_sz, int delimiter, unsigned long
                *psamples += 1;
        }
 
-       free(read_buf);
-       free(new_buf);
+       buf_del(&read_buf);
+       buf_del(&new_buf);
 
        return 0;
 }
 
+/*     validate_strtoul_
+       Wrap strtoul and check for success.
+*/
+static
+unsigned long validate_strtoul_(const char *str, unsigned int *invalid) {
+       char *ep;
+       unsigned long num;
+
+       errno = 0;
+       num = strtoul(str, &ep, 0);
+       if (errno
+       ||  ! (optarg && *optarg != '\0' && *ep == '\0') ) {
+               *invalid += 1;
+               NOTIFY_ERROR("could not parse '%s' as a number: %s", str, strerror(errno));
+       }
+       return num;
+}
 
 int main(int argc, char *argv[]) {
        struct sigaction sa;
        char *status_filename = NULL;
        reservoir_t reservoir;
        unsigned long num_samples = 0;
+       unsigned int options_error = 0;
        int c;
 
        while ( (c = getopt(argc, argv, "hvb:d:D:i:j:s:n:r:")) != EOF ) {
@@ -295,8 +330,7 @@ int main(int argc, char *argv[]) {
                                break;
 
                        case 'b':
-                               options_.read_buf_sz = strtoul(optarg, NULL, 0);
-                               /* XXX: validate */
+                               options_.read_buf_sz = validate_strtoul_(optarg, &options_error);
                                break;
 
                        case 'd':
@@ -307,11 +341,11 @@ int main(int argc, char *argv[]) {
                                break;
 
                        case 'i':
-                               options_.reservoir_grow_per = strtoul(optarg, NULL, 0);
+                               options_.reservoir_grow_per = validate_strtoul_(optarg, &options_error);
                                break;
 
                        case 'j':
-                               options_.reservoir_grow_double_per = strtoul(optarg, NULL, 0);
+                               options_.reservoir_grow_double_per = validate_strtoul_(optarg, &options_error);
                                break;
 
                        case 's':
@@ -319,7 +353,7 @@ int main(int argc, char *argv[]) {
                                break;
 
                        case 'n':
-                               options_.reservoir_sz = strtoul(optarg, NULL, 0);
+                               options_.reservoir_sz = validate_strtoul_(optarg, &options_error);
                                break;
 
                        case 'r':
@@ -336,6 +370,10 @@ int main(int argc, char *argv[]) {
                }
        }
 
+       if (options_error) {
+               exit(EX_USAGE);
+       }
+
 #if 0
        /* zero-or-more arguments required */
        /* if that ever changes... */