Re: [libvirt] [PATCH 5/7] Add virLogSource variables to all source files

On 03/03/2014 12:18 PM, Daniel P. Berrange wrote: You know, generating the patch with git's '-Ofile' containing globs to float the important files first into the diff makes review a bit easier. Big, but I don't see any way to break it down. Idea for a followup patch - instead of having VIR_FROM_THIS hard-coded as a macro, could we instead inline it as an argument to VIR_LOG_INIT() which populates another member of the static struct? Then all the error logging functions would read it out of the struct as a single argument, instead of the current setup of getting an argument for both the struct and the VIR_FROM_THIS macro expansion as two separate arguments. If you respin the patch series, it might be nice to do the all-file-touching patch only once, rather than having to do it in two pieces. Isn't an unused variable the sign of a file with no log messages? On the other hand, ease of maintenance says it is easier to always have the logging framework in place than it is to turn it on/off per file as we edit messages in or out of a file. Would the compiler warning also go away if you used: static ATTRIBUTE_UNUSED virLogSource virlogSelf = {...}; without needing the use of a verify()? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org