[PATCH] #ifdef code cleanup

Andrew Bartlett abartlet at samba.org
Fri Nov 23 21:42:05 UTC 2018


On Fri, 2018-11-23 at 15:33 +0100, Andreas Schneider wrote:
> On Friday, 23 November 2018 09:25:23 CET Andrew Bartlett via samba-technical 
> wrote:
> > On Fri, 2018-11-23 at 10:06 +1300, Andrew Bartlett via samba-technical
> > 
> > wrote:
> > > On Thu, 2018-11-22 at 21:32 +0100, Andreas Schneider wrote:
> > > > On Thursday, 22 November 2018 19:36:57 CET Andrew Bartlett via
> > > > samba-technical> > 
> > > > wrote:
> > > > > 
> > > > > This looks good, but honestly I'm a bit nervous.  Any chance you have
> > > > > done a build somehow forcing -E into the gcc line and compared the
> > > > > results before/after?
> > > > 
> > > > This is mostly from CFLAGS="-Wundef"
> > > 
> > > Sure, it's not the detection method I'm worried about, it is that large
> > > patch sets like this can easily slip in typo or similar and cause the
> > > condition to no longer match.
> > > 
> > > A way to guard against that is to demonstrate that the build produces
> > > the same output with and without the patch.
> > > 
> > > When I make big changes to our selftest system I ensure that the same
> > > tests (or if different, I can account for the difference) still get
> > > printed in the output stream, to ensure we don't silently miss tests.
> > > 
> > > Likewise I'm pretty sure I did checking of the config.h results during
> > > the waf upgrade.
> > > 
> > > Perhaps build with eg CC="my-cc.sh" that compiles with gcc -E and then
> > > again with a full gcc, to get at the CPP output?
> > 
> > I tried and failed to build that way with the attached.  No idea why it
> > doesn't work.  So this isn't a reasonable request.  If someone else can
> > eyeball the whole series and check it is correct, I'll be happy.
> 
> Here is a list produced with the attach script. I've added comments where they 
> are checked, but the check fails so do not show up at all in config.h or we 
> have #ifdef in the code and no configure check for it, which means it is dead 
> code.

I did a test on my host with the script I suggested last night, fixed
up to finally work.

I had to revert:

s3: Remove unsused MMAP_BLACKLIST ifdef checks
lib:replace: Check if HAVE_DECL_ENVIRON is defined first

(these can be reasoned about independently). 

To make it give the (almost) identical results you see below.

Thanks for you work on this.

(No, this isn't a review, but it is an answer to the question I posed
above). 

Thanks,

Andrew Bartlett

-- 
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba

-------------- next part --------------
A non-text attachment was scrubbed...
Name: gcc-E
Type: text/x-python
Size: 542 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20181124/ed694bb6/gcc-E-0001.py>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: asn-ifdef.diff
Type: text/x-patch
Size: 2130547 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20181124/ed694bb6/asn-ifdef-0001.bin>


More information about the samba-technical mailing list