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 :| -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list