[Top][All Lists]

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

Re: [Qemu-block] [RFC] error: auto propagated local_err

From: Vladimir Sementsov-Ogievskiy
Subject: Re: [Qemu-block] [RFC] error: auto propagated local_err
Date: Thu, 19 Sep 2019 13:17:50 +0000

19.09.2019 16:03, Kevin Wolf wrote:
> Am 19.09.2019 um 14:00 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> 19.09.2019 12:17, Kevin Wolf wrote:
>>> Am 18.09.2019 um 19:10 hat Eric Blake geschrieben:
>>>> On 9/18/19 8:02 AM, Vladimir Sementsov-Ogievskiy wrote:
>>>>> + */
>>>>> +#define MAKE_ERRP_SAFE(errp) \
>>>>> +g_auto(ErrorPropagationStruct) (__auto_errp_prop) = {.errp = (errp)}; \
>>>>> +if ((errp) == NULL || *(errp) == error_abort || *(errp) == error_fatal) 
>>>>> { \
>>>>> +    (errp) = &__auto_errp_prop.local_err; \
>>>>> +}
>>>> Not written to take a trailing semicolon in the caller.
>>>> You could even set __auto_errp_prop unconditionally rather than trying
>>>> to reuse incoming errp (the difference being that error_propagate() gets
>>>> called more frequently).
>>> I think this difference is actually a problem.
>>> When debugging things, I hate error_propagate(). It means that the Error
>>> (specifically its fields src/func/line) points to the outermost
>>> error_propagate() rather than the place where the error really happened.
>>> It also makes error_abort completely useless because at the point where
>>> the process gets aborted, the interesting information is already lost.
>>> So I'd really like to restrict the use of error_propagate() to places
>>> where it's absolutely necessary. Unless, of course, you can fix these
>>> practical problems that error_propagate() causes for debugging.
>>> In fact, in the context of Greg's series, I think we really only need to
>>> support hints for error_fatal, which are cases that users are supposed
>>> to see. We should exclude error_abort in MAKE_ERRP_SAFE() because these
>>> are things that are never supposed to happen. A good stack trace is more
>>> important there than adding a hint to the message.
>> Interesting, that to handle error_append_hint problem, we don't need to
>> create local_err in case of errp==NULL either..
>> So, possibly, we need the following steps:
>> 1. implement MAKE_ERRP_SAFE_FOR_HINT (which only leave "*(errp) == 
>> error_fatal" in the if condition
>> 2. rebase Greg's series on it, to fix hints for fatal errors
>> 3. implement MAKE_ERRP_SAFE_FOR_DEREFERENCE (which only leave "(errp) == 
>> NULL" in the if condition
>> 4. convert all (almost all) local_err usage to use 
>>      fix problem with error_abort (and also drop a lot of calls of 
>> error_propagate)
>> 5. merely convert "void func(.., errp)" to "int func(.., errp)" and drop 
>>      magic, together with dereferencing.
> Long macro names, but as the parameter will always only be "errp", it
> fits easily on a line, so this is fine.

Yes, I wanted to stress their meaning in plan..

Other variants, I can imagine:



> I think I like this plan.
> Kevin

Best regards,

reply via email to

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