On 9/19/19 8:03 AM, Kevin Wolf wrote: >> >> 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 >> MAKE_ERRP_SAFE_FOR_DEREFERENCE, which will >> fix problem with error_abort (and also drop a lot of calls of >> error_propagate)
Why do we need two macros? A single MAKE_ERRP_SAFE that covers both NULL and &error_fatal at the same time is sufficient. You can then always use append_hint and/or *errp as you wish, and the error_propagate() call is only used in two situations: caller passed NULL (so we free the error as ignored), caller passed &error_fatal (so the hint got appended before we propagate it to flag a more useful message). >> 5. merely convert "void func(.., errp)" to "int func(.., errp)" and drop >> MAKE_ERRP_SAFE_FOR_DEREFERENCE() >> magic, together with dereferencing. That's orthogonal, but may also be useful cleanups. I don't think we need to drop the macro, though. > > Long macro names, but as the parameter will always only be "errp", it > fits easily on a line, so this is fine. > > I think I like this plan. > > Kevin > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
