On Thu, Dec 20, 2018 at 7:00 AM Yauheni Kaliuta
<yauheni.kaliuta@xxxxxxxxxx> wrote:
>
> Hi, Lucas!
>
> >>>>> On Tue, 18 Dec 2018 16:02:28 -0800, Lucas De Marchi wrote:
>
> > Allow to test outputs when they don't match exactly, but should follow
> > some regex patterns. This can be used when the info we are printing is
> > randomized or depends on kernel configuration.
> > ---
> > testsuite/testsuite.c | 104 +++++++++++++++++++++++++++++++++++++++++-
> > testsuite/testsuite.h | 6 +++
> > 2 files changed, 108 insertions(+), 2 deletions(-)
>
> > diff --git a/testsuite/testsuite.c b/testsuite/testsuite.c
> > index 550c711..db36324 100644
> > --- a/testsuite/testsuite.c
> > +++ b/testsuite/testsuite.c
> > @@ -20,6 +20,7 @@
> > #include <fcntl.h>
> > #include <getopt.h>
> > #include <limits.h>
> > +#include <regex.h>
> > #include <stdarg.h>
> > #include <stdio.h>
> > #include <stdlib.h>
> > @@ -293,8 +294,98 @@ static int check_activity(int fd, bool activity, const char *path,
> > #define BUFSZ 4096
> > struct buffer {
> > char buf[BUFSZ];
> > + unsigned int head;
> > };
>
> > +
> > +static bool cmpbuf_regex_one(const char *pattern, const char *s)
> > +{
> > + _cleanup_(regfree) regex_t re = { };
> > +
> > + return !regcomp(&re, pattern, REG_EXTENDED|REG_NOSUB) &&
> > + !regexec(&re, s, 0, NULL, 0);
> > +}
> > +
> > +/*
> > + * read fd and fd_match, checking the first matches the regex of the second,
> > + * line by line
> > + */
> > +static bool cmpbuf_regex(const struct test *t, const char *prefix,
> > + int fd, int fd_match, struct buffer *buf,
> > + struct buffer *buf_match)
> > +{
> > + char *p, *p_match;
> > + int done = 0, done_match = 0, r;
> > +
> > + r = read(fd, buf->buf + buf->head, sizeof(buf->buf) - buf->head - 1);
>
> Why do you need -1 if you are replacing existing '\n' and using
> mem* functions with explicit size?
so buf + head never points outside of the buffer and so avoid a
segfault. It could be rather replace by sanity checks on top of the
function though. I'm changing that.
>
>
> > + if (r <= 0)
> > + return true;
> > +
> > + buf->head += r;
> > +
> > + /*
> > + * Process as many lines as read from fd and that fits in the buffer -
> > + * it's assumed that we we get N lines from fd, we should be able to
> ^^^^^^^^^ extra "we"?
fixed
>
> > + * get the same amount from fd_match
> > + */
> > + for (;;) {
> > + p = memchr(buf->buf + done, '\n', buf->head - done);
> > + if (!p)
> > + break;
> > + *p = 0;
>
> Would '\0' be better to emphasize the string end?
I use them interchangeably... no preference. I'll change it here though.
>
> > +
> > + p_match = memchr(buf_match->buf + done_match, '\n',
> > + buf_match->head - done_match);
> > + if (!p_match) {
> > + /* pump more data from file */
> > + r = read(fd_match, buf_match->buf + buf_match->head,
> > + sizeof(buf_match->buf) - buf_match->head - 1);
> > + if (r <= 0) {
> > + ERR("could not read match fd %d\n", fd_match);
> > + return false;
> > + }
> > + buf_match->head += r;
> > + p_match = memchr(buf_match->buf + done_match, '\n',
> > + buf_match->head - done_match);
> > + if (!p_match) {
> > + ERR("could not find match line from fd %d\n", fd_match);
> > + return false;
> > + }
> > + }
> > + *p_match = 0;
>
> ditto.
fixed
Thanks,
Lucas De Marchi
>
> > +
> > + if (!cmpbuf_regex_one(buf_match->buf + done_match, buf->buf + done)) {
> > + ERR("Output does not match pattern on %s:\n", prefix);
> > + ERR("pattern: %s\n", buf_match->buf + done_match);
> > + ERR("output : %s\n", buf->buf + done);
> > + return false;
> > + }
> > +
> > + done = p - buf->buf + 1;
> > + done_match = p_match - buf_match->buf + 1;
> > + }
> > +
> > + /*
> > + * Prepare for the next call: anything we processed we remove from the
> > + * buffer by memmoving the remaining bytes up to the beginning
> > + */
> > +
> > + if (done) {
> > + if (buf->head - done)
> > + memmove(buf->buf, buf->buf + done, buf->head - done);
> > + buf->head -= done;
> > + }
> > +
> > + if (done_match) {
> > + if (buf_match->head - done_match)
> > + memmove(buf_match->buf, buf_match->buf + done_match,
> > + buf_match->head - done_match);
> > + buf_match->head -= done_match;
> > + }
> > +
> > + return true;
> > +}
> > +
> > /* read fd and fd_match, checking they match exactly */
> > static bool cmpbuf_exact(const struct test *t, const char *prefix,
> > int fd, int fd_match, struct buffer *buf,
> > @@ -428,6 +519,7 @@ static bool test_run_parent_check_outputs(const struct test *t,
>
> > for (i = 0; i < fdcount; i++) {
> > int fd = *(int *)ev[i].data.ptr;
> > + bool ret;
>
> > if (ev[i].events & EPOLLIN) {
> > int fd_match;
> > @@ -452,9 +544,17 @@ static bool test_run_parent_check_outputs(const struct test *t,
>
> > buf_match = buf + 1;
>
> > - if (!cmpbuf_exact(t, prefix, fd, fd_match,
> > - buf, buf_match))
> > + if (t->output.regex)
> > + ret = cmpbuf_regex(t, prefix, fd, fd_match,
> > + buf, buf_match);
> > + else
> > + ret = cmpbuf_exact(t, prefix, fd, fd_match,
> > + buf, buf_match);
> > +
> > + if (!ret) {
> > + err = -1;
> > goto out;
> > + }
> > } else if (ev[i].events & EPOLLHUP) {
> > if (epoll_ctl(fd_ep, EPOLL_CTL_DEL, fd, NULL) < 0) {
> > ERR("could not remove fd %d from epoll: %m\n", fd);
> > diff --git a/testsuite/testsuite.h b/testsuite/testsuite.h
> > index 2b31483..7ed96bf 100644
> > --- a/testsuite/testsuite.h
> > +++ b/testsuite/testsuite.h
> > @@ -88,6 +88,12 @@ struct test {
> > /* File with correct stderr */
> > const char *err;
>
> > + /*
> > + * whether to treat the correct files as regex to the real
> > + * output
> > + */
> > + bool regex;
> > +
> > /*
> > * Vector with pair of files
> > * key = correct file
> > --
> > 2.20.0
>
>
> --
> WBR,
> Yauheni Kaliuta
--
Lucas De Marchi
