Re: [libvirt] [PATCHv3 1/3] util: capabilities detection for dnsmasq

Should we also follow the lead of tests/qemuhelpdata/ and do an actual capture from released dnsmasq binaries, to ensure that our parser for those files matches our artificial dnsmasqCaps objects? More on this below...[1] Spurious whitespace change? Then again, we haven't been very consistent on one vs. two blank lines between .h files. Given Dan's recent rename of 'struct qemud_driver *driver' to the friendlier 'virQEMUDriverPtr driver', should we do a similar change here? But if so, separate it to its own patch. Long line; worth wrapping? You asked me about this one on IRC. If libvirt had a single copyright holder, then I'd say this is okay (it follows the convention[2] given by the FSF, where if a repository is publicly available, then touching any one file in that repository counts as a copyrightable claim on the package as a whole, and that all files in the package may claim the same range of copyright years as the package as a whole, even for files that were not touched in a particular year). It is a bit more questionable here (since we have NOT required anyone to assign copyright, libvirt has a multitude of copyright holders), but I still think you can get away with this change (Red Hat does indeed own the copyrights on a vast majority of the code base, and has made copyrightable contributions in every single year in this range). I guess it all boils down to how likely we are to ever try and defend the copyright in court (FSF has a much easier job of that task, thanks to them requiring copyright assignment; whereas with libvirt, it is up to individual copyright holders over their individual portions of the overall work to decide what they would defend in court). [2] https://www.gnu.org/prep/maintain/maintain.html#Copyright-Notices "To update the list of year numbers, add each year in which you have made nontrivial changes to the package. (Here we assume you’re using a publicly accessible revision control server, so that every revision installed is also immediately and automatically published.) When you add the new year, it is not required to keep track of which files have seen significant changes in the new year and which have not. It is recommended and simpler to add the new year to all files in the package, and be done with it for the rest of the year." "You can use a range (‘2008-2010’) instead of listing individual years (‘2008, 2009, 2010’) if and only if: 1) every year in the range, inclusive, really is a “copyrightable” year that would be listed individually; and 2) you make an explicit statement in a README file about this usage." Could we use virSkipSpaces() instead of open-coding this? Slightly more compact as: p = STRSKIP(buf, DNSMASQ_VERSION_STR); if (!p) goto fail; Slightly more efficient as: p = strchrnul(buf, '\n') Comment attached to the wrong function. [1] Here is where I'm worried that we might want to also test sample actual output, in separate data files (which are also tagged as immune to syntax checking). But that would be a separate test (much as qemuhelptest is separate from qemuxml2argvtest), and could therefore be delayed to a later patch, so I'm okay with the amount of testing done in this particular test. I had some points you might want to clean up, but they are all minor (no logic bugs, just style or micro-optimizations), so I'm okay giving ACK. I've seen enough patches from you that I trust you to make the cleanups and/or post followups without needing to see a v4 just to address my findings.