[PATCH] #ifdef code cleanup

Ralph Böhme slow at samba.org
Sat Nov 24 09:58:06 UTC 2018


On Sat, Nov 24, 2018 at 10:42:05AM +1300, Andrew Bartlett via samba-technical wrote:
>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.

the gcc -E trick is really nice. Can we put that somewhere in the wiki? If you 
have a suggestion where to put it, I'll happily volunteer to make the change. :)

-slow

-- 
Ralph Boehme, Samba Team                https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/
GPG-Fingerprint   FAE2C6088A24252051C559E4AA1E9B7126399E46



More information about the samba-technical mailing list