All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Tokarev <mjt@tls.msk.ru>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: QEMU Trivial <qemu-trivial@nongnu.org>,
	QEMU Developers <qemu-devel@nongnu.org>,
	Le Tan <tamlokveer@gmail.com>
Subject: Re: [Qemu-trivial] [Qemu-devel] [PULL 21/23] bsd-user: replace fprintf(stderr, ...) with error_report()
Date: Mon, 02 Jun 2014 16:08:14 +0400	[thread overview]
Message-ID: <538C692E.9090000@msgid.tls.msk.ru> (raw)
In-Reply-To: <CAFEAcA9wDmQmed6YNrFy5jyRTN2mCD3JOnk=f-eiqxGx5iycXA@mail.gmail.com>

01.06.2014 02:39, Peter Maydell wrote:
> On 26 May 2014 08:20, Michael Tokarev <mjt@tls.msk.ru> wrote:
>> From: Le Tan <tamlokveer@gmail.com>
>>
>> Replace fprintf(stderr,...) with error_report() in files bsd-user/*.
>> The trailing "\n"s of the @fmt argument have been removed
>> because @fmt of error_report() should not contain newline.
>>
>> Signed-off-by: Le Tan <tamlokveer@gmail.com>
>> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> 
> Was this patch tested? Building on FreeBSD the compiler
> complains:
> "warning: implicit declaration of function 'error_report' is invalid in C99"
> 
> because none of these bsd-user files include a header which
> gives a prototype for error_report. Also, these are just
> straightforward reporting of command line errors, and I
> think that, like the linux-user code, we should handle
> these in the obvious way by printing to stderr. There's no
> need to drag in the error-handling framework for this,
> especially since user-mode doesn't have the "maybe we
> need to send this to the monitor" issues system emulation
> does.

Umm.  it is a Very Good call.

I applied it and actually tried to compile-check it, on kFreeBSD.
Compile went successfully, and I was satisfied, until I figured
that my kFreeBSD test script only builds qemu-system.  So I went
back and enabled this, and actually found the issue and even
fixed it locally, by adding the #includes.  While doing this, I
wondered, why such a basic/common subsystem is not included in
there to start with, and so isn't used?  Maybe this is something
which shouldn't be done?

But I got distracted from all this due to other issues, and when
I come back I just pushed the whole thing without noticing the
added #includes aren't committed, and forgetting about my doubts.
That's what you get when doing stuff in a hurry.

> In short, I think we need to revert this commit
> (1fba509527beb).

Yes, that's what I think too.  Should I send a formal patch
submission, or is `git revert' easy enough?  Even with my
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
if needed?

Thanks,

/mjt



WARNING: multiple messages have this Message-ID (diff)
From: Michael Tokarev <mjt@tls.msk.ru>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: QEMU Trivial <qemu-trivial@nongnu.org>,
	QEMU Developers <qemu-devel@nongnu.org>,
	Le Tan <tamlokveer@gmail.com>
Subject: Re: [Qemu-devel] [Qemu-trivial] [PULL 21/23] bsd-user: replace fprintf(stderr, ...) with error_report()
Date: Mon, 02 Jun 2014 16:08:14 +0400	[thread overview]
Message-ID: <538C692E.9090000@msgid.tls.msk.ru> (raw)
In-Reply-To: <CAFEAcA9wDmQmed6YNrFy5jyRTN2mCD3JOnk=f-eiqxGx5iycXA@mail.gmail.com>

01.06.2014 02:39, Peter Maydell wrote:
> On 26 May 2014 08:20, Michael Tokarev <mjt@tls.msk.ru> wrote:
>> From: Le Tan <tamlokveer@gmail.com>
>>
>> Replace fprintf(stderr,...) with error_report() in files bsd-user/*.
>> The trailing "\n"s of the @fmt argument have been removed
>> because @fmt of error_report() should not contain newline.
>>
>> Signed-off-by: Le Tan <tamlokveer@gmail.com>
>> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> 
> Was this patch tested? Building on FreeBSD the compiler
> complains:
> "warning: implicit declaration of function 'error_report' is invalid in C99"
> 
> because none of these bsd-user files include a header which
> gives a prototype for error_report. Also, these are just
> straightforward reporting of command line errors, and I
> think that, like the linux-user code, we should handle
> these in the obvious way by printing to stderr. There's no
> need to drag in the error-handling framework for this,
> especially since user-mode doesn't have the "maybe we
> need to send this to the monitor" issues system emulation
> does.

Umm.  it is a Very Good call.

I applied it and actually tried to compile-check it, on kFreeBSD.
Compile went successfully, and I was satisfied, until I figured
that my kFreeBSD test script only builds qemu-system.  So I went
back and enabled this, and actually found the issue and even
fixed it locally, by adding the #includes.  While doing this, I
wondered, why such a basic/common subsystem is not included in
there to start with, and so isn't used?  Maybe this is something
which shouldn't be done?

But I got distracted from all this due to other issues, and when
I come back I just pushed the whole thing without noticing the
added #includes aren't committed, and forgetting about my doubts.
That's what you get when doing stuff in a hurry.

> In short, I think we need to revert this commit
> (1fba509527beb).

Yes, that's what I think too.  Should I send a formal patch
submission, or is `git revert' easy enough?  Even with my
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
if needed?

Thanks,

/mjt

  reply	other threads:[~2014-06-02 12:08 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-26  7:19 [Qemu-trivial] [PULL 00/23] Trivial patches for 2014-05-26 Michael Tokarev
2014-05-26  7:19 ` [Qemu-devel] " Michael Tokarev
2014-05-26  7:19 ` [Qemu-trivial] [PULL 01/23] libcacard: g_malloc cleanups Michael Tokarev
2014-05-26  7:19   ` [Qemu-devel] " Michael Tokarev
2014-05-26  7:19 ` [Qemu-trivial] [PULL 02/23] iohandler.c: Properly initialize sigaction struct Michael Tokarev
2014-05-26  7:19   ` [Qemu-devel] " Michael Tokarev
2014-05-26  7:19 ` [Qemu-trivial] [PULL 03/23] nbd: Close socket on negotiation failure Michael Tokarev
2014-05-26  7:19   ` [Qemu-devel] " Michael Tokarev
2014-05-26  7:19 ` [Qemu-trivial] [PULL 04/23] nbd: Miscellaneous typo fixes Michael Tokarev
2014-05-26  7:19   ` [Qemu-devel] " Michael Tokarev
2014-05-26  7:19 ` [Qemu-trivial] [PULL 05/23] configure: Automatically select GTK+ 3.0 if GTK+ 2.0 is unavailable Michael Tokarev
2014-05-26  7:19   ` [Qemu-devel] " Michael Tokarev
2014-05-26  7:19 ` [Qemu-trivial] [PULL 06/23] bswap.h: Rename ldl_p, stl_p, etc to ldl_he_p, stl_he_p, etc Michael Tokarev
2014-05-26  7:19   ` [Qemu-devel] " Michael Tokarev
2014-05-26  7:19 ` [Qemu-trivial] [PULL 07/23] jazz_led: Add missing break in switch case Michael Tokarev
2014-05-26  7:19   ` [Qemu-devel] " Michael Tokarev
2014-05-26  7:19 ` [Qemu-trivial] [PULL 08/23] pci: move dereferencing of root only after verifying valid root pointer Michael Tokarev
2014-05-26  7:19   ` [Qemu-devel] " Michael Tokarev
2014-05-26  7:19 ` [Qemu-trivial] [PULL 09/23] arch_init: replace fprintf(stderr, ...) with error_report() Michael Tokarev
2014-05-26  7:19   ` [Qemu-devel] " Michael Tokarev
2014-05-26  7:19 ` [Qemu-trivial] [PULL 10/23] dma-helpers: avoid calling dma_bdrv_unmap() twice Michael Tokarev
2014-05-26  7:19   ` [Qemu-devel] " Michael Tokarev
2014-05-26  7:19 ` [Qemu-trivial] [PULL 11/23] configure: Put tempfiles in a subdir of the build directory Michael Tokarev
2014-05-26  7:19   ` [Qemu-devel] " Michael Tokarev
2014-05-26  7:20 ` [Qemu-trivial] [PULL 12/23] vl: fix 'name' option to work with -readconfig Michael Tokarev
2014-05-26  7:20   ` [Qemu-devel] " Michael Tokarev
2014-05-26  7:20 ` [Qemu-trivial] [PULL 13/23] libcacard/vscclient: Bury some dead code Michael Tokarev
2014-05-26  7:20   ` [Qemu-devel] " Michael Tokarev
2014-05-26  7:20 ` [Qemu-trivial] [PULL 14/23] libcacard: Plug memory leaks around vreader_get_reader_list() Michael Tokarev
2014-05-26  7:20   ` [Qemu-devel] " Michael Tokarev
2014-05-26  7:20 ` [Qemu-trivial] [PULL 15/23] libcacard/vreader: Drop broken recovery from failed assertion Michael Tokarev
2014-05-26  7:20   ` [Qemu-devel] " Michael Tokarev
2014-05-26  7:20 ` [Qemu-trivial] [PULL 16/23] libcacard/vreader: Tighten assertion to clarify intent Michael Tokarev
2014-05-26  7:20   ` [Qemu-devel] " Michael Tokarev
2014-05-26  7:20 ` [Qemu-trivial] [PULL 17/23] libcacard: Convert two leftover realloc() to GLib Michael Tokarev
2014-05-26  7:20   ` [Qemu-devel] " Michael Tokarev
2014-05-26  7:20 ` [Qemu-trivial] [PULL 18/23] libcacard/vcard_emul_nss: Drop a redundant conditional Michael Tokarev
2014-05-26  7:20   ` [Qemu-devel] " Michael Tokarev
2014-05-26  7:20 ` [Qemu-trivial] [PULL 19/23] libcacard: fix wrong array expansion logic Michael Tokarev
2014-05-26  7:20   ` [Qemu-devel] " Michael Tokarev
2014-05-26  7:20 ` [Qemu-trivial] [PULL 20/23] audio: replace fprintf(stderr, ...) with error_report() in audio Michael Tokarev
2014-05-26  7:20   ` [Qemu-devel] " Michael Tokarev
2014-05-26  7:20 ` [Qemu-trivial] [PULL 21/23] bsd-user: replace fprintf(stderr, ...) with error_report() Michael Tokarev
2014-05-26  7:20   ` [Qemu-devel] " Michael Tokarev
2014-05-31 22:39   ` [Qemu-trivial] " Peter Maydell
2014-06-02 12:08     ` Michael Tokarev [this message]
2014-06-02 12:08       ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
2014-06-02 12:34       ` [Qemu-trivial] [Qemu-devel] " Peter Maydell
2014-06-02 12:34         ` [Qemu-devel] [Qemu-trivial] " Peter Maydell
2014-06-02 13:09     ` [Qemu-trivial] [Qemu-devel] " Markus Armbruster
2014-06-02 13:09       ` Markus Armbruster
2014-06-02 13:16       ` [Qemu-trivial] " Michael Tokarev
2014-06-02 13:16         ` Michael Tokarev
2014-05-26  7:20 ` [Qemu-trivial] [PULL 22/23] net: cadence_gem: Fix top comment Michael Tokarev
2014-05-26  7:20   ` [Qemu-devel] " Michael Tokarev
2014-05-26  7:20 ` [Qemu-trivial] [PULL 23/23] libcacard: remove useless initializers Michael Tokarev
2014-05-26  7:20   ` [Qemu-devel] " Michael Tokarev
2014-05-27 22:30 ` [Qemu-trivial] [Qemu-devel] [PULL 00/23] Trivial patches for 2014-05-26 Peter Maydell

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=538C692E.9090000@msgid.tls.msk.ru \
    --to=mjt@tls.msk.ru \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-trivial@nongnu.org \
    --cc=tamlokveer@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.