On Wed, Oct 29, 2008 at 01:55:50PM +0100, Daniel Veillard wrote:
On Wed, Oct 29, 2008 at 11:44:31AM +0000, Daniel P. Berrange wrote:
This patch moves the definition for virRaiseError/virReportError out of the internal.h file, and into a newly created virterror.h file, to match
Hum, i was looking at this, yes using a new haeader makes sense, I will also move the libvirt error initialization to virterror.c
But the problem is that we already have a virterror.h header which is the public one, I think adding a second one internally with the same name is a recipe for confusion. Let's name the internal one error.h (or virt_error.h) but not the same.
naming of the virterror.c file where the implementation lives. Most code has to be updated to include virterror.h. This patch also drops the leading __ prefix on these two functions - it was pointless, since we never export it from the .so, not even for libvirtd use.
The motivation for this change is to get clarity over our modularization. Every file in our source tree ultimately includes internal.h, but not everything is linking to the virterror.c file which includes the impl of virRaiseError. So it is good to avoid defining functions in header files which aren't available to those including the header.
I think the name clash between the private and public header is more problematic than having a slightly different name for the .c and .h virterror.c implements the entry points for the public header virterror.h, like libvirt.c implements the entry points for the public header libvirt.h . having a second internel error header separate from internal.h is fine but we should just avoid the name clash.
+1 for renaming to error.h or virt_error.h
Having a private virt_error.h and public virterror.h is just as confusing as having both called virterror.h So, how about calling the internal one simply virterror_internal.h 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 :|