From: Cao jin <caoj.fnst@cn.fujitsu.com>
To: Marcel Apfelbaum <marcel@redhat.com>, qemu-devel@nongnu.org
Cc: mst@redhat.com, jasowang@redhat.com,
Markus Armbruster <armbru@redhat.com>,
alex.williamson@redhat.com, hare@suse.de, dmitry@daynix.com,
pbonzini@redhat.com, jsnow@redhat.com, kraxel@redhat.com
Subject: Re: [Qemu-devel] [PATCH v3] Add param Error ** for msi_init()
Date: Thu, 31 Mar 2016 16:09:23 +0800 [thread overview]
Message-ID: <56FCDB33.5010604@cn.fujitsu.com> (raw)
In-Reply-To: <56FB95C3.9020508@redhat.com>
On 03/30/2016 05:00 PM, Marcel Apfelbaum wrote:
> On 03/30/2016 07:10 AM, Cao jin wrote:
>
> Hi,
>
>>
>> Yes, I should add more hint message. I don`t quite understand about:
>>
>> /have a "warning only" error type so the reporting party can decide to
>> issue a warning or to fail/
>>
>> Do you mean still using VMW_WRPRN or using error_append_hint? It seems
>> VMW_WRPRN should only work in debug time? and if user should see this
>> error message, should use error_report_err ?
>
> No, it is not related to VMW_WRPRN. On this subject, those are debugging
> warnings
> and we should keep them the same as before.
>
ok
> About the "warning only" error type: if an error is returned, the caller
> assumes that the initialization failed and it will return with failure.
> That means
> that you cannot return a non-null error if you want the process to
> finish OK.
>
> This is why you had to:
> - report the error (even if this function should not be a reporter
> because it receives an Error parameter)
> - empty the error: so why use error at all, right?
>
> My point is if the caller had a way to check what kind of error this is
> and act accordingly, it would be nicer. But again, this is out of the
> scope of this patch, only some thoughts.
>
I see, and agree.
>>
>>>
>>
>> see what I was told before:
>> http://lists.nongnu.org/archive/html/qemu-trivial/2015-10/msg00116.html
>> http://lists.nongnu.org/archive/html/qemu-trivial/2015-10/msg00123.html
>>
>> Actually I am ok with both sides. I just stop sending coding style
>> patch since then:)
>
> Oh, I hate when it happens to me, tho different opinions, how can you
> deal with that, right ? :)
>
>>
>> I don`t know, coding style & indentation patch really matters or is
>> just a personal taste thing?
>
> Coding style and indentation *are important* IMHO.
>
Totally, absolutely agree
> For this case, what I would do is put the new lines and comments in a
> separate patch,\
> but send it with *the same* series, not through trivial list, saying
> something like:
> "fixed some code style problems while resolving the ... problem".
>
OK
--
Yours Sincerely,
Cao jin
prev parent reply other threads:[~2016-03-31 8:07 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-28 10:44 [Qemu-devel] [PATCH v3] Add param Error ** for msi_init() Cao jin
2016-03-29 20:56 ` Marcel Apfelbaum
2016-03-30 4:10 ` Cao jin
2016-03-30 9:00 ` Marcel Apfelbaum
2016-03-31 8:09 ` Cao jin [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=56FCDB33.5010604@cn.fujitsu.com \
--to=caoj.fnst@cn.fujitsu.com \
--cc=alex.williamson@redhat.com \
--cc=armbru@redhat.com \
--cc=dmitry@daynix.com \
--cc=hare@suse.de \
--cc=jasowang@redhat.com \
--cc=jsnow@redhat.com \
--cc=kraxel@redhat.com \
--cc=marcel@redhat.com \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
/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).