On Wed, Oct 08, 2008 at 02:17:45PM -0400, Cole Robinson wrote:
The attached patch is a more complete cut at unifying error reporting among the different source files. Most files have their own custom Error function, each with varying degrees of functionality, and lots of code duplication.
The attached patch adds a helper function to virterror.c that centralizes all this logic, and redefines each local error function as a macro that calls out to the helper. The fixed files now offer printf style reporting, and the macros pass off function name, file name, and line number of the reported error (currently not used, but handy to have).
This change centralizes probably 90% of the error calls. Some files have pretty custom error functions that don't map easily to the helper, or call __virRaiseError directly in a number of places, so I skipped these for now. Eventually we should move all these edge cases over so any src/ wide error changes will be pretty easy to make.
Ok.
The one thing we lose here is that some some places were logging info in the err{1,2,3} and int{1,2} fields of raised error. I don't consider this a loss: even where it was used, it was used inconsistently and rarely for anything that would be useful to a user. In the places where the data had some value, I included it in the error string.
Agreed - it had no documented use either.
One side question: is src/xmlrpc.{c,h} even used? And if not, can it be removed?
It is not used at this time. It was for the hypothetical alternative Xen driver which talks XML-RPC which we never got around to writing. Perhaps it really is time to kill it -we can get it back from CVS if ever required. ACK to this patch Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|