Il 18/10/2013 19:59, Max Reitz ha scritto: > 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.
error_propagate is not obsolete. It is particularly pervasive in generated code. You can and should skip error_propagate if you are tail-calling another function. In this case, the extra if/error_propagate pair is useless, makes the code less clear and adds 3-4 useless lines of code. If you have an alternative way to see whether an error occurred (typically based on the return value: <0 if it is int, NULL if it is a pointer), it is fine to use it instead of error_propagate, because error_propagate adds some complexity to the logic. It is also fine to use error_propagate; whatever makes the code simpler. However, the converse is not true. If you have a function that returns void and takes an Error*, it is not okay to make it return int or bool for the sake of avoiding error_propagate. There are also cases where you have a return value, but you cannot use it to ascertain whether an error occurred. For example, NULL may be a valid return value even when no error occurs. In such case, you have to use error_propagate. In the end, I agree with Kevin: "If in doubt, use local_err". Tail calls should be the only case where local_err is clearly unnecessary, any other case needs to be considered specifically. Paolo