[PATCH] #ifdef code cleanup

[Gary Lockyer]

Looks good to me RB+
Pushing to autobuild.

Gary

On 28/11/18 21:04, Andreas Schneider via samba-technical wrote:
> On Friday, 23 November 2018 22:42:05 CET 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.
>>
>> Thanks for you work on this.
>>
>> (No, this isn't a review, but it is an answer to the question I posed
>> above).
> 
> 
> Review still needed ...
> 
> 
> Anyone?
> 
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20181129/a66f4e8b/signature.sig>