Peter Crosthwaite <peter.crosthwa...@xilinx.com> writes: > On Fri, Nov 29, 2013 at 1:03 AM, Markus Armbruster <arm...@redhat.com> wrote: >> Paolo Bonzini <pbonz...@redhat.com> writes: >> >>> Il 28/11/2013 14:23, Igor Mammedov ha scritto: >>>> > object_property_set(Foo, bar, "baz", &abort_on_err); >>>> >>>> that is just another way to put burden on caller, instead of doing it >>>> in one place. >>> >>> It's also much more self-documenting. >>> >>> The problem is that there is one very good case where you want the >>> silent-don't-care behavior: when you don't care about the exact error, >>> and you can figure out whether there was an error from the returned >>> value of the function. This doesn't apply to object_property_set of >>> course, but it is the reason why NULL Error* has silent-don't-care behavior. >>> >>> Now, let's look at the alternatives: >>> >>> * keep silent don't care >>> + consistent >>> + predictable >>> - not always handy >>> >>> * only modify object_property_set >>> + mostly does the right thing >>> - inconsistent with other Error* functions >>> - inconsistent with _nofail functions >>> >>> * Peter's alternative >>> + self-documenting >>> + consistent >>> + predictable >>> >>> * make Error* mandatory for all void functions >>> + consistent >>> + almost predictable (because in C you can ignore return values) >>> - not necessarily does the right thing (e.g. cleanup functions) >>> - requires manual effort to abide to the policy >>> >>> I vote for Peter's proposal, or for adding object_property_set_nofail. >>> No particular preference. >>> >>> Another variant: modify object_property_set to add a g_warning. I think >>> it's fine. It reduces the inconsistency, and still simplifies debugging. >> >> I like Peter's proposal, provided we use it to get rid of the _nofail >> pattern. >> >> Second preference is adding another _nofail wrapper. >> > > So this issue with _nofail is that if you are doing it properly, every > API needed both normal and _nofail version of every function. APIs > generally have no bussiness dictating failure policy so by extension, > normal and _nofail should exist for every API that accepts an Error *. > With my proposal, its fixed once, in one place and we can torch all > the _nofail boilerplate all over the tree as you have suggested. > > These is another subtle advantage to my proposal, and that is that > assertions can happen at the point of failure in the offending API, > not after the fact in the caller. If the caller does an > assert_no_error, then the abort() happens after return from the API > call. This makes debugging awkward cause the stack frames into the API > call where the issue actually occured are lost. Whereas if error_set > does the abort() you will get all stack frames into the API call where > the issue occured when gdb traps the abort().
To make further progress, we need a patch. Care to cook one up?
