qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 1/5] migration: split hmp_savevm to do_savevm an


From: Denis V. Lunev
Subject: Re: [Qemu-devel] [PATCH 1/5] migration: split hmp_savevm to do_savevm and hmp_savevm wrapper
Date: Fri, 8 Jan 2016 20:59:26 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0

On 01/08/2016 08:54 PM, Eric Blake wrote:
On 01/08/2016 09:40 AM, Denis V. Lunev wrote:

Markus' series to add a prefixing notation would be better to use here
(although I didn't check if he caught this one in that series already):
https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg03495.html
this series is not yet merged. I think that we could do this refactoring
later on.
This thing could be considered independent. Anyway, this series has its
own value
and it takes a lot of time to push it in. Could we do  error setting
improvement later on?
I don't care who rebases on top of the other, but maybe Markus will have
an opinion when he gets back online next week.

why we have to wait with this set due to this reason?
One of you will have to rebase on the other - either you wait for
Markus' error_prepend to go in and you use it, or your patch goes in and
Markus updates his error_prepend patch to cover your additional instance
that will be benefitted by it.  I don't care which, and the timing is
really up to the maintainers and how fast they send pull requests.

The code with error_prepend and current code are BOTH
correct. One is a bit shorter then other. Yes, it would
be nice to switch to it, but why this should be done in
this set?
Exactly, we're saying the same things.

+    if (local_err != NULL) {
I would have just written 'if (local_err) {'; but that's minor style.
from my point of view explicit != NULL exposes that local_err is a
pointer rather than a boolean value.
But the code base already overwhelmingly relies on C's implicit
conversion of pointer to a boolean context, as it requires less typing;
being verbose doesn't make the code base any easier to read.  However,
since HACKING doesn't say one way or the other, I won't make you change.

I do not understand your last words.

I am not agitating you with one approach or another. This
is a reason why I am writing code this way. The code written
this way looks better to me. This code is NEW and this does
not contradict any written rule in coding style policy.

If the code is working and correct, can we just move on with it?
Once again, we are saying the same thing.  I pointed out a cosmetic
issue, but one where I do not have a strong enough leg to stand on to
force you to change your style, so what you did is fine as is.

ok. perfect to be on the same page :)

I'll promise to switch to error_prepend code when it will be
merged. I hope that v4 of the set is good enough to
proceed.

Den



reply via email to

[Prev in Thread] Current Thread [Next in Thread]