[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: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 1/5] migration: split hmp_savevm to do_savevm and hmp_savevm wrapper |
Date: |
Fri, 8 Jan 2016 10:54:21 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 |
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.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature