[Top][All Lists]

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

Re: [Qemu-devel] [PATCH for-4.0 v8 6/7] qemu_thread_create: propagate th

From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH for-4.0 v8 6/7] qemu_thread_create: propagate the error to callers to handle
Date: Wed, 19 Dec 2018 11:11:56 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)

Fei Li <address@hidden> writes:

> On 12/17/2018 03:29 PM, Fei Li wrote:
>> On 12/13/2018 03:26 PM, Markus Armbruster wrote:
>>> There's a question for David Gibson inline.  Please search for /ppc/.
>>> Fei Li <address@hidden> writes:
>>>> Make qemu_thread_create() return a Boolean to indicate if it succeeds
>>>> rather than failing with an error. And add an Error parameter to hold
>>>> the error message and let the callers handle it.
>>> The "rather than failing with an error" is misleading.  Before the
>>> patch, we report to stderr and abort().  What about:
>>>      qemu-thread: Make qemu_thread_create() handle errors properly
>>>      qemu_thread_create() abort()s on error.  Not nice.  Give it a
>>>      return value and an Error ** argument, so it can return success /
>>>      failure.
>> A nice commit-amend! Thanks!
>>> Still missing from the commit message then: how you update the callers.
>> Yes, agree. I think the-how should also be noted here, like
>> - propagating the err to callers whose call trace already have the
>> Error paramater;
>> - just add an &error_abort for qemu_thread_create() and make it a
>> "TODO: xxx";
>>> Let's see below.
> According to your below comment and suggestion, I make a summary for
> the second paragraph for the commit message, please help to review,
> thanks. :)
> /* ...The first paragraph and the middle blank... */
> And let's update qemu_thread_create()'s callers by
> - setting an error on qemu_thread_create() failure for callers that
>   set an error on failure;
> - reporting the error and returning failure for callers that return
>   an error code on failure;
> - reporting the error and setting some state for callers that just
>   report errors and choose not to continue on.
> - passing &error_abort for qemu_thread_create() for callers that
>   can't return failure, and marking a "TODO: " for further change.
> Have a nice day
> Fei

If you split the patch so that the first part makes all callers pass
&error_abort, the first part's commit message becomes much simpler, and
the subsequent parts' commit messages should be pretty simple to write,

reply via email to

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