[SSSD] [PATCH 3/4] Make DEBUG macro variadic

Simo Sorce simo at redhat.com
Fri Oct 11 12:53:11 UTC 2013


On Fri, 2013-10-11 at 11:09 +0300, Nikolai Kondrashov wrote:
> On 10/10/2013 11:27 PM, Simo Sorce wrote:
> > On Thu, 2013-10-10 at 15:46 -0400, Stephen Gallagher wrote:
> >> -----BEGIN PGP SIGNED MESSAGE-----
> >> Hash: SHA1
> >>
> >> On 10/10/2013 10:07 AM, Nikolai Kondrashov wrote:
> >>> Make DEBUG macro accept variable number of arguments, thus removing
> >>> the need to wrap the format string and its arguments into parens
> >>> on invocation.
> >>
> >> Nack.
> >>
> >> Please do us a favor and split this into two patches, one that makes
> >> the change to the macro definition and one that bulk-updates the
> >> existing messages. Having a 2.2MB patch file with a few small changes
> >> buried in it somewhere is very difficult to review. I'm not confident
> >> I can locate the real changes amidst the find-replace noise.
> >
> > I would not want to consider such a patch in any case. The churn is just
> > too extraordinarily big and I do not see enough value to warrant it.
> >
> > The patch is so immense that is it impossible to review, so we would
> > have to make a leap of faith in assuming every single change did not
> > introduce errors.
> 
> I'll try to reduce the amount of faith required. Maybe try a semantic patch
> [1], or do some metrics on the code and the changes.
> 
> > I am not against 'simplifying' the debug macro if possible, but then it
> > should probably be done in a backwards compatible way and debug
> > statements changed in time, just like we did when we switched from mere
> > numbers to error type definitions.
> >
> > For example we could call the new macro DBG, an start using it in place
> > of DEBUG in new code. DEBUG would be just a wrapper around DBG that
> > maintains the old interface.
> 
> I would advise against that. This will hurt maintainability, and I think
> having two ways to specify debug level is harmful as well.  This is how
> software rot happens. It is better to have a way to verify that such changes
> don't break anything (tests?) and do a change globally, or not change anything
> at all.
> 
> However, this is of course entirely up to you and I won't push this further as
> I'm only doing it for a sport, while waiting for Beaker job queue to unclog.
> 
> Sincerely,
> Nick
> 
> [1] http://coccinelle.lip6.fr/sp.php

I seriously like the simplification, but I am dead against such a
monstrous patch that touches *all* the code.

Although not ideal I am perfectly fine with changing code gradually like
we did (and are still doing) with the debug numbers.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York




More information about the sssd-devel mailing list