Re: [PATCH v4 00/31] error: auto propagated local_err

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

08.10.2019 13:09, Vladimir Sementsov-Ogievskiy wrote:
> 08.10.2019 12:39, Greg Kurz wrote:
>> On Tue, 8 Oct 2019 08:41:08 +0000
>> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:
>>
>>> 08.10.2019 10:30, Markus Armbruster wrote:
>>>> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:
>>>>
>>>>> Hi all!
>>>>>
>>>>> Here is a proposal of auto propagation for local_err, to not call
>>>>> error_propagate on every exit point, when we deal with local_err.
>>>>>
>>>>> There are also two issues with errp:
>>>>>
>>>>> 1. error_fatal & error_append_hint/error_prepend: user can't see this
>>>>> additional info, because exit() happens in error_setg earlier than info
>>>>> is added. [Reported by Greg Kurz]
>>>>
>>>> How is this series related to Greg's "[PATCH 00/17] Fix usage of
>>>> error_append_hint()"?  Do we need both?
>>>
>>> These series is a substitution for Greg's. Still, there are problems with
>>> automation, which Greg pointed in 21/31, and I don't know what to do next.
>>>
>>> May be, just continue to review patches and fix them by hand. May be try to
>>> improve automation...
>>>
>>
>> The feeling I have after working on my series is that the lines that deal
>> with errors are mixed up with the functional code in a variety of ways.
>> That makes it very difficult if not impossible to come with code patterns
>> suitable for a 100% automated solution IMHO.
>>
>> My questioning is more around the semantics of error_fatal actually. What
>> does passing &error_fatal gives us over passing &local_err and calling
>> error_report_err()+exit(), apart from breaking error_append_hint() and
>> error_prepend() ?
> 
> As I understand, the only benefit is one line instead of four:
> 
> func(..., &error_fatal);
> 
> instead of
> 
> func(..., &local_err);
> if (local_err) {
>      exit(1);
> }
> 
> But, keeping in mind all difficulties about these series... We can consider
> conversion error_fatal -> local_err too. It seems simple to do with a coccinelle
> script, I can send another automatic series to look at it.
> 
> 
> Hmm, some ideas around this:
> 
> 1. We can generate _fatal versions of functions by python script (we'll call it from Makefile, we have a lot of generated code anyway).
> 
> and convert
> func(..., &local_err); to
> 
> func_fatal(...);
> 
> 2. Use macro like
> 
> #define FATAL(func, ...) do { Error *__fatal_err = NULL; func(__VA_ARGS__ __VA_OPT(,), &__fatal_err); if (__fatal_err) { error_report(__fatal_err); exit(1); } } while (0)
> 
> and convert
> func(..., &local_err); to
> 
> FATAL(func, ...);

Now, I think, that what these series do is better, as
func(..., &fatal_err) and one macro invocation at function start
is better than too many generated functions.

-- 
Best regards,
Vladimir