On Fri, Jan 4, 2019 at 8:28 AM Yauheni Kaliuta
<yauheni.kaliuta@xxxxxxxxxx> wrote:
>
> Hi, Lucas!
>
> Just in case, ACK.
pushed, thanks.
Lucas De Marchi
>
> >>>>> On Thu, 3 Jan 2019 13:07:44 -0800, Lucas De Marchi wrote:
>
> > Move functionality to compare the exact output to a separate function
> > and allocate one buffer per output/match pair. This will allow us to
> > extend this to allow other types of comparisons. Since now we are using
> > heap-allocated buffer, keep the buffer allocation to the caller, so we
> > don't have to allocate and free it on every invocation. It also avoids
> > the different comparison functions to have to deal with it.
> > ---
> > testsuite/testsuite.c | 124 ++++++++++++++++++++++++------------------
> > 1 file changed, 70 insertions(+), 54 deletions(-)
>
> > diff --git a/testsuite/testsuite.c b/testsuite/testsuite.c
> > index 8512b56..508bbd3 100644
> > --- a/testsuite/testsuite.c
> > +++ b/testsuite/testsuite.c
> > @@ -290,13 +290,64 @@ static int check_activity(int fd, bool activity, const char *path,
> > return -1;
> > }
>
> > -static inline bool test_run_parent_check_outputs(const struct test *t,
> > - int fdout, int fderr, int fdmonitor, pid_t child)
> > +#define BUFSZ 4096
> > +struct buffer {
> > + char buf[BUFSZ];
> > +};
> > +
> > +/* 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,
> > + struct buffer *buf_match)
> > +{
> > + int r, rmatch, done = 0;
> > +
> > + r = read(fd, buf->buf, sizeof(buf->buf) - 1);
> > + if (r <= 0)
> > + /* try again later */
> > + return true;
> > +
> > + /* read as much data from fd_match as we read from fd */
> > + for (;;) {
> > + rmatch = read(fd_match, buf_match->buf + done, r - done);
> > + if (rmatch == 0)
> > + break;
> > +
> > + if (rmatch < 0) {
> > + if (errno == EINTR)
> > + continue;
> > + ERR("could not read match fd %d\n", fd_match);
> > + return false;
> > + }
> > +
> > + done += rmatch;
> > + }
> > +
> > + buf->buf[r] = '\0';
> > + buf_match->buf[r] = '\0';
> > +
> > + if (t->print_outputs)
> > + printf("%s: %s\n", prefix, buf->buf);
> > +
> > + if (!streq(buf->buf, buf_match->buf)) {
> > + ERR("Outputs do not match on %s:\n", prefix);
> > + ERR("correct:\n%s\n", buf_match->buf);
> > + ERR("wrong:\n%s\n", buf->buf);
> > + return false;
> > + }
> > +
> > + return true;
> > +}
> > +
> > +static bool test_run_parent_check_outputs(const struct test *t,
> > + int fdout, int fderr, int fdmonitor,
> > + pid_t child)
> > {
> > struct epoll_event ep_outpipe, ep_errpipe, ep_monitor;
> > int err, fd_ep, fd_matchout = -1, fd_matcherr = -1;
> > bool fd_activityout = false, fd_activityerr = false;
> > unsigned long long end_usec, start_usec;
> > + _cleanup_free_ struct buffer *buf_out = NULL, *buf_err = NULL;
>
> > fd_ep = epoll_create1(EPOLL_CLOEXEC);
> > if (fd_ep < 0) {
> > @@ -320,6 +371,7 @@ static inline bool test_run_parent_check_outputs(const struct test *t,
> > ERR("could not add fd to epoll: %m\n");
> > goto out;
> > }
> > + buf_out = calloc(2, sizeof(*buf_out));
> > } else
> > fdout = -1;
>
> > @@ -340,6 +392,7 @@ static inline bool test_run_parent_check_outputs(const struct test *t,
> > ERR("could not add fd to epoll: %m\n");
> > goto out;
> > }
> > + buf_err = calloc(2, sizeof(*buf_err));
> > } else
> > fderr = -1;
>
> > @@ -374,76 +427,39 @@ static inline bool test_run_parent_check_outputs(const struct test *t,
> > }
>
> > for (i = 0; i < fdcount; i++) {
> > - int *fd = ev[i].data.ptr;
> > + int fd = *(int *)ev[i].data.ptr;
>
> > if (ev[i].events & EPOLLIN) {
> > - ssize_t r, done = 0;
> > - char buf[4096];
> > - char bufmatch[4096];
> > int fd_match;
> > + struct buffer *buf, *buf_match;
> > + const char *prefix;
>
> > - /*
> > - * compare the output from child with the one
> > - * saved as correct
> > - */
> > -
> > - r = read(*fd, buf, sizeof(buf) - 1);
> > - if (r <= 0)
> > - continue;
> > -
> > - if (*fd == fdout) {
> > + if (fd == fdout) {
> > fd_match = fd_matchout;
> > + buf = buf_out;
> > fd_activityout = true;
> > - } else if (*fd == fderr) {
> > + prefix = "STDOUT";
> > + } else if (fd == fderr) {
> > fd_match = fd_matcherr;
> > + buf = buf_err;
> > fd_activityerr = true;
> > + prefix = "STDERR";
> > } else {
> > ERR("Unexpected activity on monitor pipe\n");
> > err = -EINVAL;
> > goto out;
> > }
>
> > - for (;;) {
> > - int rmatch = read(fd_match,
> > - bufmatch + done, r - done);
> > - if (rmatch == 0)
> > - break;
> > -
> > - if (rmatch < 0) {
> > - if (errno == EINTR)
> > - continue;
> > - err = -errno;
> > - ERR("could not read match fd %d\n",
> > - fd_match);
> > - goto out;
> > - }
> > -
> > - done += rmatch;
> > - }
> > + buf_match = buf + 1;
>
> > - buf[r] = '\0';
> > - bufmatch[r] = '\0';
> > -
> > - if (t->print_outputs)
> > - printf("%s: %s\n",
> > - fd_match == fd_matchout ? "STDOUT:" : "STDERR:",
> > - buf);
> > -
> > - if (!streq(buf, bufmatch)) {
> > - ERR("Outputs do not match on %s:\n",
> > - fd_match == fd_matchout ? "STDOUT" : "STDERR");
> > - ERR("correct:\n%s\n", bufmatch);
> > - ERR("wrong:\n%s\n", buf);
> > - err = -1;
> > + if (!cmpbuf_exact(t, prefix, fd, fd_match,
> > + buf, buf_match))
> > 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);
> > + if (epoll_ctl(fd_ep, EPOLL_CTL_DEL, fd, NULL) < 0) {
> > + ERR("could not remove fd %d from epoll: %m\n", fd);
> > }
> > - *fd = -1;
> > + *(int *)ev[i].data.ptr = -1;
> > }
> > }
> > }
> > --
> > 2.20.0
>
>
> --
> WBR,
> Yauheni Kaliuta
--
Lucas De Marchi
