From: Lucas De Marchi <lucas.de.marchi@gmail.com>
To: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>,
linux-modules <linux-modules@vger.kernel.org>
Subject: Re: [PATCH v2 1/3] testsuite: split out function to compare outputs exactly
Date: Fri, 4 Jan 2019 12:58:16 -0800 [thread overview]
Message-ID: <CAKi4VALP-JEGX7_W3pxcvxrY7YM=OokRrZihrb8+4+3hE6+T4g@mail.gmail.com> (raw)
In-Reply-To: <xuny1s5sjtb0.fsf@redhat.com>
On Fri, Jan 4, 2019 at 8:28 AM Yauheni Kaliuta
<yauheni.kaliuta@redhat.com> 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
prev parent reply other threads:[~2019-01-04 20:58 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20190103210746.20048-1-lucas.demarchi@intel.com>
2019-01-03 21:07 ` [PATCH v2 2/3] testsuite: add support for testing output against regex Lucas De Marchi
2019-01-03 21:07 ` [PATCH v2 3/3] testsuite: move --show-exports test to use regex Lucas De Marchi
2019-01-04 16:12 ` [PATCH v2 1/3] testsuite: split out function to compare outputs exactly Yauheni Kaliuta
2019-01-04 20:58 ` Lucas De Marchi [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAKi4VALP-JEGX7_W3pxcvxrY7YM=OokRrZihrb8+4+3hE6+T4g@mail.gmail.com' \
--to=lucas.de.marchi@gmail.com \
--cc=linux-modules@vger.kernel.org \
--cc=lucas.demarchi@intel.com \
--cc=yauheni.kaliuta@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).