On 06.05.2016 20:03, John Ferlan wrote:
On 05/04/2016 08:33 AM, Michal Privoznik wrote:
All these patches will be squashed into one, but I've split them into multiple because of easier review.
Michal Privoznik (4): qemu_monitor_json: Follow refactor qemu_monitor_json: Follow refactor qemu_monitor_json: Follow refactor qemu_monitor_json: Follow refactor
src/qemu/qemu_monitor_json.c | 673 ++++++++++++++++++++++--------------------- 1 file changed, 349 insertions(+), 324 deletions(-)
Not a deal breaker, but most code has:
if (qemuMonitorJSONCheckError(cmd, reply) < 0) goto cleanup;
ret = 0; cleanup:
while a few instances [1] have:
ret = qemuMonitorJSONCheckError(cmd, reply);
cleanup:
IDC either way since the result is the same, but I suppose they "could" be all done the same way. I'd probably lean towards the latter, but since you already went with the former in commit '7884d089' it's probably best to keep using that.
[1] qemuMonitorJSONSetMigrationCompression qemuMonitorJSONBlockCommit qemuMonitorJSONInjectNMI [2] qemuMonitorJSONSendKey [2] qemuMonitorJSONSetMigrationCapability
[2] both instances use the same HMP call attempt to return ret - they could follow the qemuMonitorJSONSetCPU model...
Oh. Thanks for catching that. I'll fix those too.
Finally, I see the comments move 4 spaces to the right in qemuMonitorJSONSetObjectProperty? (patch 4)... Was that a remnant or expected?
While working on this I didn't bother with adjusting the code that much initially. After that I've fired up my code alignment key shortcut over each function I've touched. I will leave the comments as they were.
ACK series (your call if you want to adjust [2] now or in a followup patch. it's trivial enough as long as you remember to also remove the {} around what would become "one line" if then else statements).
Thanks, pushed now. Michal