[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: Eric Blake
Subject: Re: [Qemu-block] [RFC] error: auto propagated local_err
Date: Wed, 18 Sep 2019 13:05:32 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0

On 9/18/19 12:46 PM, Vladimir Sementsov-Ogievskiy wrote:

>>> +/*
>>> + * Third variant:
>>> + *   Pros:
>>> + *     - simpler movement for functions which don't have local_err yet
>>> + *       the only thing to do is to call one macro at function start.
>>> + *       This extremely simplifies Greg's series
>>> + *   Cons:
>>> + *     - looks like errp shadowing.. Still seems safe.
>>> + *     - must be after all definitions of local variables and before any
>>> + *       code.
>> Why?  I see no reason why it can't be hoisted earlier than other
>> declarations, and the only reason to not sink it after earlier code that
>> doesn't touch errp would be our coding standards that frowns on
>> declaration after code.
> Hmm, I thought compiler would warn about mixing code and definitions.
> Seems that gcc don't care, so it's OK.

C89 required all definitions before code, but that's historical.
Meanwhile, we require a compiler that supports C99 as well as at least
the __attribute__((cleanup)) extension (gcc and clang qualify, nothing
else really does, but no one has been complaining).  And C99 requires
compiler support for intermixing definitions (in part because c++ did it
first, then gcc allowed that as an extension in C89); so it's been
permissible to intermix code and declarations for more than 20 years
now.  The only reason we don't do it more is because of habits and
aesthetics, rather than necessity.

>> This is actually quite cool. 
> I glad to see that you like my favorite variant)
>> And if you get rid of your insistence that
>> it must occur after other variable declarations, you could instead
>> easily automate that any function that has a parameter 'Error **errp'
>> then has a MAKE_ERRP_SAFE(errp); as the first line of its function body
>> (that becomes something that you could grep for, rather than having to
>> use the smarts of coccinelle).
>> Or if we want to enforce consistency on the parameter naming, even go with:
>> #define MAKE_ERRP_SAFE() \
>> g_auto(ErrorPropagationStruct) (__auto_errp_prop) = {.errp = errp}; \
>> errp = &__auto_errp_prop.local_err
> I am for

So now to wait for other comments.

>>> +     */
>>> +    MAKE_ERRP_SAFE(errp);
>>> +
>>>       if (filename) {
>>>           ret = qemu_gluster_parse_uri(gconf, filename);
>>>           if (ret < 0) {
>> This is sweet - you can safely use '*errp' in the rest of the function,
>> without having to remember a second error name - while the caller can
>> still pass NULL or error_abort as desired.
>> And I still think we can probably get Coccinelle to help make the
>> conversions, both of using this macro in any function that has an Error
>> **errp parameter, as well as getting rid of local_err declarations and
>> error_propagate() calls rendered redundant once this macro is used.
> Thanks! And sorry for dirty draft.

It was titled RFC, after all :)

Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

reply via email to

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