From: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
To: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: linux-modules@vger.kernel.org
Subject: Re: [PATCH 1/3] testsuite: split out function to compare outputs exactly
Date: Thu, 20 Dec 2018 12:38:35 +0200 [thread overview]
Message-ID: <xuny36qslbys.fsf@redhat.com> (raw)
In-Reply-To: <20181219000229.10793-1-lucas.demarchi@intel.com> (Lucas De Marchi's message of "Tue, 18 Dec 2018 16:02:27 -0800")
Hi, Lucas!
>>>>> On Tue, 18 Dec 2018 16:02:27 -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.
> ---
> testsuite/testsuite.c | 124 ++++++++++++++++++++++++------------------
> 1 file changed, 70 insertions(+), 54 deletions(-)
> diff --git a/testsuite/testsuite.c b/testsuite/testsuite.c
> index 8512b56..550c711 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)
> +{
Not so clear (either from description) without the next patch,
why the buffers are not local for the function.
> + int r, rmatch, done = 0;
> +
> + r = read(fd, buf, sizeof(buf->buf) - 1);
buf->buf?
> + 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
next prev parent reply other threads:[~2018-12-20 10:38 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-19 0:02 [PATCH 1/3] testsuite: split out function to compare outputs exactly Lucas De Marchi
2018-12-19 0:02 ` [PATCH 2/3] testsuite: add support for testing output against regex Lucas De Marchi
2018-12-20 10:41 ` Yauheni Kaliuta
2019-01-03 20:41 ` Lucas De Marchi
2018-12-20 10:44 ` Yauheni Kaliuta
2019-01-03 20:50 ` Lucas De Marchi
2019-01-03 21:06 ` Yauheni Kaliuta
2019-01-03 21:14 ` Lucas De Marchi
2018-12-19 0:02 ` [PATCH 3/3] testsuite: move --show-exports test to use regex Lucas De Marchi
2018-12-20 10:38 ` Yauheni Kaliuta [this message]
2019-01-03 20:09 ` [PATCH 1/3] testsuite: split out function to compare outputs exactly Lucas De Marchi
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=xuny36qslbys.fsf@redhat.com \
--to=yauheni.kaliuta@redhat.com \
--cc=linux-modules@vger.kernel.org \
--cc=lucas.demarchi@intel.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).