Re: [libvirt] [PATCH 04/11] qemu_security: Kill code duplication
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(a)redhat.com>
>>> ---
>>> 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