Re: [Qemu-devel] [PATCH v2] mirror: drop local_err in mirror_complete

[Kevin Wolf <kwolf@redhat.com>]

Am 18.10.2013 um 19:59 hat Max Reitz geschrieben:
> Then there's still the problem that I'm not the one who introduced
> error_propagate. Someone obviously intended some purpose for it, so
> even if it doesn't make a difference now (and my RFC is unneeded),
> I'd still use it to propagate errors (instead of passing the error
> pointer). My point being that there *is* a function for propagating
> errors and I think we should therefore use it. The current qemu code
> seems to alternate between the two alternatives, although using
> error_propagate seems more common to me (at least, that was the
> result when I looked through the code trying to decide whether to
> use it or not).
> 
> Generally, we need a proper discussion about whether error_propagate
> is obsolete or not. Afterwards, we can adapt the current codebase to
> the result of that discussion; but until then, I oppose changing
> existing code without actual need to do so but personal preference.
> 
> Max
> 
> 
> PS: I wrote my error_propagate RFC in part because I was
> disappointed about how much of a no-op error_propagate actually is
> and was trying to change that. ;-)

It's not a no-op. The point is that the caller can pass NULL as errp if
it isn't interested in the error message. If you do anything else with
errp than just passing it to other functions - most commonly a
error_is_set() check - you need to make sure that you use a local Error
variable and propagate it. Otherwise, if the caller passed NULL, your
error path would never get executed.

So in this specific case, not having a local_err and error_propagate()
does work, but it's not generally safe. If in doubt, use local_err.

Regarding this patch, I'm not sure it's useful.

Kevin