On Fri, Nov 29, 2013 at 5:56 PM, Markus Armbruster <arm...@redhat.com> wrote: > 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? >
Done. Regards, Peter
