On 15 June 2015 at 16:18, Luiz Capitulino <lcapitul...@redhat.com> wrote: > On Sat, 13 Jun 2015 16:20:51 +0200 > Markus Armbruster <arm...@redhat.com> wrote: > >> Error classes other than ERROR_CLASS_GENERIC_ERROR should not be used >> in new code. Hiding them in QERR_ macros makes new uses hard to spot. >> Fortunately, there's just one such macro left. Eliminate it with this >> coccinelle semantic patch: >> >> @@ >> expression EP, E; >> @@ >> -error_set(EP, QERR_DEVICE_NOT_FOUND, E) >> +error_set(EP, ERROR_CLASS_DEVICE_NOT_FOUND, "Device '%s' not found", E) > > This is a bit minor, but I think I'd have created a new function instead, > say error_set_enodev(). This avoids all the duplication. But I'm not asking > you to change, as the patch is good and this can be done in the future if > we so want.
The thing about that kind of generic set-an-error function is that it encourages people to use it rather than providing an error message that's more specific and helpful for the particular situation. That might not be a problem in this patch (I haven't read it), but I mention it because I have a patch onlist elsewhere which undoes a bit of "generic error based on an errno" in favour of being more specific about why something didn't work. thanks -- PMM
