Re: [Qemu-devel] [PATCH v1 2/8] tests: Replace fprintf(stderr, "*\n" with error_report()

["Emilio G. Cota" <cota@braap.org>]

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 <cota@braap.org> 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