X-Git-Url: http://git.squeep.com/?p=reservoir_sample;a=blobdiff_plain;f=reservoir_sample.c;fp=reservoir_sample.c;h=50a1235642f9d2528fa7db8da096ec829486339f;hp=1c64f6fe5598179395241f48c643df05c4b94a45;hb=ee831dbcdf31c3d1205bc321a8bdbeeb151abe19;hpb=c8ff35a649a7df3d400221fc76131bff195a158b diff --git a/reservoir_sample.c b/reservoir_sample.c index 1c64f6f..50a1235 100644 --- a/reservoir_sample.c +++ b/reservoir_sample.c @@ -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 -- returns samples [default: %zu]\n" "\t-d -- use as input and output delimiter [default: '\\%03hho']\n" "\t-r -- read randomness from [default: '%s']\n" - "\t-b -- set input buffer size [default: %zu]\n" + "\t-b -- input buffer size [default: %zu]\n" "\t-i -- grow reservoir by 1 for every input samples (0 inhibits behavior) [default: %lu]\n" - "\t-h -- double the reservoir growth interval every input samples (0 inhibits behavior) [default: %lu]\n " + "\t-j -- double the reservoir growth interval every input samples (0 inhibits behavior) [default: %lu]\n " "\t-s -- USR1 signals will write reservoir contents to 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... */