On Thu, Sep 17, 2015 at 05:31:28PM +0200, Michal Privoznik wrote:
So, now we have VIR_ENUM_DECL() and VIR_ENUM_IMPL() macros. They will define a pair of functions for you to convert from and to certain enum. Unfortunately, their usage is slightly cumbersome:
int tmp; virMyAwesome ma;
if ((tmp = virMyAwesomeTypeFromString(str)) < 0) { virReportError(); goto cleanup; }
ma = tmp;
Ideally, we could avoid using the dummy @tmp variable:
virMyAwesome ma;
if (virMyAwesomeEnumFromString(str, &ma, "Unable to convert '%s'", str) < 0) goto cleanup;
So, the first @str is string to convert from. @ma should point to the variable, where result is stored. Then, the third argument is the error message to be printed if conversion fails, followed by all the necessary arguments (message is in printf format).
I'm not seeing a hugely compelling reason to pass in the error message string every time we parse an enum from a string. This is no better than current code, where every caller has a potentially different error message reported for the same scenario. I'd like to see virMyAwesome ma; if (virMyAwesomeEnumFromString(str, &ma) < 0) goto cleanup; We could introduce an explicit VIR_ERR_INVALID_ENUM_VALUE error code, and a fixed error message to go with it.
-# define VIR_ENUM_DECL(name) \ - const char *name ## TypeToString(int type); \ - int name ## TypeFromString(const char*type); +# define VIR_ENUM_DECL(name) \ + const char *name ## TypeToString(int type); \ + int name ## TypeFromString(const char*type); \ + int name ## EnumFromString(const char *str, int *result, \ + const char *fmt, ...) \ + ATTRIBUTE_FMT_PRINTF(3, 4);
It'd be nice to avoid generating two different FromString methods at the same time too. I understand you probably did that to avoid needing to convert all callers at once. How about we have a VIR_ENUM_DECL_ERR macro generate the new format only, and just convert all callers at a time, for any single enum. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|