On 06/02/2015 05:51 AM, Markus Armbruster wrote: >>>> The most confusing part about this is that you have &error_abort almost >>>> everywhere, but the function doesn't actually abort on error, but rather >>>> returns a negative error code and leaves errp alone. >>> >>> True. The function contract spells it out, which hopefully reduces the >>> confusion somewhat. >> >> Except that you don't enforce the contract; I suggested adding >> assert(!*errp) at the right place in the two conversions. >> >>> >>> Would you find NULL less confusing than &error_abort? >> >> NULL says to ignore errors, &error_abort says to diagnose errors as >> programming bugs. If we know we aren't going to have an error, I prefer >> diagnosing coding bugs. > > You prefer &error_abort, Kevin prefers NULL, so I need to figure out > what I prefer to break the tie :) > > I think we can agree on these two rules on Error ** arguments: > > R1: When caller doesn't care whether the callee sets an error, it should > pass NULL. > > R2: When a caller relies on the callee not setting an error, it should > pass &error_abort.
Yes, these two rules cover the current state of the art. > > R1 applies, R2 does not, thus we should pass NULL. > > The case for &error_abort requires a third rule: > > Proposed R3: When a caller knows that the callee won't set an error, it > may pass &error_abort to document this knowledge even when it doesn't > actually rely on it (thus R2 doesn't apply). This is an exception to > R1. Or, as I explored in another message, if the caller passes NULL, but we then turn it to &error_abort locally, to enforce that the callback does not set an error for either success or failure. > > To keep things simple, I lean towards rejecting R3 and passing NULL. > > Opinions? At this point I'm leaning towards simplicity - pass NULL, and not worth modifying the contract (passing NULL does not need to get transformed into error_abort). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
