[libvirt] [PATCH v2 02/42] util: handle missing switch enum cases

Daniel P. Berrangé berrange at redhat.com
Tue Feb 20 11:25:24 UTC 2018


On Tue, Feb 20, 2018 at 12:19:15PM +0100, Andrea Bolognani wrote:
> On Tue, 2018-02-20 at 09:54 +0000, Daniel P. Berrangé wrote:
> > > > +        case VIR_CONF_LAST:
> > > >         default:
> > > > +            virReportError(VIR_ERR_INTERNAL_ERROR,
> > > > +                           _("Unexpected conf value type %d"), val->type);
> > > >             return -1;
> > > 
> > > All these errors are presumably dead code that we only keep around in
> > > case we broke something in other parts of the code.
> > > 
> > > Do we need specific user-friendly translated errors? Since we log the
> > > function name as well, something like: "unhandled enum value %d" would
> > > do.
> > 
> > The function name only gets into the logs - not the error reporting,
> > so if someone does get an error raised, I don't want it to be a totally
> > generic message that could come from literally anywhere in the codebase.
> 
> Yesterday I argued in a different thread that it would be better
> to include the enum name in the error message, since that's useful
> information for developers whereas users 1) should never see this
> kind of error to begin with and 2) when they do, their only course
> of action is reporting the issue anyway.

How about we standard it via a special API

   virReportErrorEnumRange(virDomainControllerModelUSB, val->type);

and map this through to a VIR_ERR_ENUM_RANGE error code, with a fixed
string format.

   "Value '%d' out of range for enum %s"

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|




More information about the libvir-list mailing list