[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH v1 2/8] tests: Replace fprintf(stde
From: |
Emilio G. Cota |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH v1 2/8] tests: Replace fprintf(stderr, "*\n" with error_report() |
Date: |
Thu, 28 Sep 2017 13:53:51 -0400 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Wed, Sep 27, 2017 at 16:08:50 -0700, Alistair Francis wrote:
> On Mon, Sep 25, 2017 at 5:55 PM, Emilio G. Cota <address@hidden> wrote:
> > On Mon, Sep 25, 2017 at 17:08:48 -0700, Alistair Francis wrote:
> >> diff --git a/tests/atomic_add-bench.c b/tests/atomic_add-bench.c
> >> index caa1e8e689..41ba1600c0 100644
> >> --- a/tests/atomic_add-bench.c
> >> +++ b/tests/atomic_add-bench.c
> >> @@ -29,7 +29,7 @@ static const char commands_string[] =
> >> static void usage_complete(char *argv[])
> >> {
> >> fprintf(stderr, "Usage: %s [options]\n", argv[0]);
> >> - fprintf(stderr, "options:\n%s\n", commands_string);
> >> + fprintf(stderr, "options:\n%s", commands_string);
> >> }
> >
> > We do want that trailing \n, unless we move it to commands_string.
>
> Yeah, this was a mistake. It should have swapped to error_report().
>
> >
> > Also, I think using error_report here would be confusing -- this is a
> > standalone
> > test program with as little QEMU-specific knowledge as possible (QemuThreads
> > are used for portability); using error_report here is confusing (this is not
> > an error).
>
> Do you mean in all tests/ sub directory or just a few certain cases?
I was talking specifically about tests/atomic-add-bench and tests/qht-bench,
because I know those well. I don't have a good grasp on everything that's under
tests/,
but I presume there are similar cases there too -- that is, standalone programs
where QEMU monitor and/or any other QEMU-specific form of logging makes no
sense,
and therefore fprintf(stderr) is the right thing to do.
Thanks,
Emilio