On 02/08/2017 01:43 PM, Peter Krempa wrote: > On Wed, Feb 08, 2017 at 13:37:48 +0100, Michal Privoznik wrote: >> On 02/08/2017 01:23 PM, Peter Krempa wrote: >>> On Wed, Feb 08, 2017 at 11:37:07 +0100, Michal Privoznik wrote: >>>> Nearly all of these functions look the same. Except for a >>>> different virSecurityManager API call. There is no need to copy >>>> paste the code when we can use macros to generate it. >>>> >>>> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> >>>> --- >>>> src/qemu/qemu_security.c | 179 ++++++++++++----------------------------------- >>>> 1 file changed, 44 insertions(+), 135 deletions(-) >>> >>> NACK, please don't partialy define function with macros. >>> >> >> Why not? What is the downside? > > You'll never be able to navigate to the body of the function or ever > find it try 'vim -t qemuSecurityRestoreHostdevLabel' or navigate to > that after that patch. > > The downside of the code being totally unreadable is way worse than a > few copied lines. > > (YU|NA)CK > VIR_ONCE_GLOBAL_INIT KVM_FEATURE_DEF DECLARE_CLASS_COMMON src/esx/* tests/* XDR_PRIMITIVE_DISSECTOR VIR_ENUM_DECL VIR_ENUM_IMPL PROBE just to name a few places where we already do what you NACKed. If using macros to construct functions is that bad, I expect you to write a patch to get rid of those. Michal -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
