Daniel P. Berrangé <berra...@redhat.com> writes: > On Thu, Sep 22, 2022 at 08:07:43AM +0200, Markus Armbruster wrote: >> Ease of use matters, too. When sticking to the rule leads to awkward >> code, we should stop and think. Should we deviate from the rule? Or >> can we avoid that by tweaking the interface? >> >> Philippe's proposed interface sticks to the rule. > > The cost is that when you see a function dosomething(true|false) as > a reader you often have no idea what the effect of true vs false is > on the behaviour of that function. You resort to looking at the > API docs and/or code. This is where C would really benefit from > having named parameters like as dosomething(ignore_errors=true|false) > is totally obvious. Anyway, I digress.
Right. Quoting myself: "If having to pass a flag turns out to to be a legibility issue, we can have wrapper functions." :) >> Another interface that does: return -1 for error, 0 for module not found >> (no error), and 1 for loaded. > > IMHO this pattern is generally easier to understand when looking at > the callers, as the fatal error scenario is always clear. > > That said I would suggest neither approach as the public facing > API. Rather stop trying to overload 3 states onto an error reporting > pattern that inherantly wants to be 2 states. Instead just have > distinct methods Like these: > bool module_load_one(const char *prefix, const char *name, Error *errp) > bool module_try_load_one(const char *prefix, const char *name, Error *errp) > > other names are available for the second, eg module_load_one_optional() module_load_one_if_there()? By the way, the "one" in "module_load_one" & friends feels redundant. When I see "module_load", I assume it loads one module. > Internally, both would call into a common helper following either > Philippe's idea, or the -1/0/1 int return value. Either is fine, > as they won't be exposed to any caller. Yup.
