(XS) RFR: 8077674: BSD build failures due to undefined macros

[David Holmes]

Hi Dmitry,

Can we get closure on this. Are you okay with:

http://cr.openjdk.java.net/~dholmes/8077674/webrev.v2/

If not please advise exactly how you think this should be fixed.

Thanks,
David

On 16/04/2015 5:32 PM, David Holmes wrote:
> On 16/04/2015 5:00 PM, Dmitry Samersoff wrote:
>> David,
>>
>>> True but "normal" programming style for mutually exclusive
>>> conditions is to use if/else style logic.
>>
>> Agree in general, but I vote for better readability/error resistance
>> here.
>
> It might be more readable in the sense it is minimalist, but IMHO it is
> less understandable to treat mutually exclusive conditions as if they
> were not mutually exclusive. I don't see the error resistance argument ??
>
>> Also, you may consider to add "sanity" block to the top of file to
>> defend to incorrect defines (multiple variants, VAR = 0 etc)
>
> Not sure what you mean but this seems to be exploding beyond the simple
> build fix I was trying to quickly get out.
>
> Thanks,
> David
>
>> -Dmitry
>>
>>
>> On 2015-04-16 09:47, David Holmes wrote:
>>> On 16/04/2015 4:43 PM, Dmitry Samersoff wrote:
>>>> David,
>>>>
>>>> As __FreeBSD__ and __OpenBSD__ etc is unlikely defined together
>>>> you don't need #else here.
>>>
>>> True but "normal" programming style for mutually exclusive conditions is
>>> to use if/else style logic.
>>>
>>> It's a third option I suppose, but I find it less appealing.
>>>
>>> Thanks,
>>> David
>>>
>>>> i.e.
>>>>
>>>> #ifdef __FreeBSD__
>>>> ...
>>>> #endif
>>>>
>>>> #ifdef __OpenBSD__
>>>> ...
>>>> #endif
>>>>
>>>> etc.
>>>>
>>>> -Dmitry
>>>>
>>>> On 2015-04-16 08:25, David Holmes wrote:
>>>>> Here's an alternate webrev based on Kim's findings in the other
>>>>> thread.
>>>>>
>>>>> http://cr.openjdk.java.net/~dholmes/8077674/webrev.v2/
>>>>>
>>>>> Basically #elif XXX is replaced with #else #ifdef XXX #endif". With
>>>>> lots
>>>>> of new lines of course.
>>>>>
>>>>> I don't know what the normal style rules are for nesting preprocessing
>>>>> conditionals. I prefer to see the # lines indented, but I know people
>>>>> like to see the real code at the indent it would be if there were no
>>>>> conditional.
>>>>>
>>>>> As I wrote here:
>>>>>
>>>>> http://mail.openjdk.java.net/pipermail/build-dev/2015-April/014801.html
>>>>>
>>>>>
>>>>> It seems to me there are two choices here:
>>>>>
>>>>> a) Use the [original] change proposed but verify that the variables
>>>>> concerned are ones that if defined will never be defined to be zero
>>>>> (which I believe to be the case but may have trouble proving :( )
>>>>>
>>>>> b) Replace the 'elif XXX' with 'else ifdef XXX' [v2]
>>>>>
>>>>> For the few cases involved here (b) seems the easier path.
>>>>>
>>>>> Comments/suggestions welcomed.
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>
>>>>>
>>>>> On 16/04/2015 11:50 AM, David Holmes wrote:
>>>>>> Still need Review for this please (with plans to backport to 8u).
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>>
>>>>>> On 15/04/2015 6:25 AM, David Holmes wrote:
>>>>>>> Hi Dmitry,
>>>>>>>
>>>>>>> On 14/04/2015 9:25 PM, Dmitry Samersoff wrote:
>>>>>>>> David,
>>>>>>>>
>>>>>>>> It's better to change
>>>>>>>>
>>>>>>>> #ifdef __APPLE__
>>>>>>>>
>>>>>>>> to
>>>>>>>>
>>>>>>>> #if defined(__APPLE__)
>>>>>>>
>>>>>>> Why? They seem 100% equivalent. Plus that would mean a lot of
>>>>>>> changes.
>>>>>>> Plus we use that pattern in many, many, many places.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> David
>>>>>>>
>>>>>>>> -Dmitry
>>>>>>>>
>>>>>>>>
>>>>>>>> On 2015-04-14 13:20, David Holmes wrote:
>>>>>>>>> webrev:
>>>>>>>>>
>>>>>>>>> http://cr.openjdk.java.net/~dholmes/8077674/webrev/
>>>>>>>>>
>>>>>>>>>    From the original error report:
>>>>>>>>>> "-Wundef option causes GCC to warn whenever it encounters an
>>>>>>>>>> identifier
>>>>>>>>>> which is not a macro in an ‘#if’. "
>>>>>>>>>>
>>>>>>>>>> Hence we should be using
>>>>>>>>>>
>>>>>>>>>> #elif defined(__FreeBSD__)
>>>>>>>>>>
>>>>>>>>>> etc in this context.
>>>>>>>>>
>>>>>>>>> I also founds similar suspect usages in os_linux.cpp and
>>>>>>>>> os_windows.cpp.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> David
>>>>>>>>
>>>>>>>>
>>>>
>>>>
>>
>>