(XS) RFR: 8077674: BSD build failures due to undefined macros
Dmitry Samersoff
dmitry.samersoff at oracle.com
Mon Apr 20 07:01:06 UTC 2015
David,
Looks good for me.
-Dmitry
On 2015-04-20 04:32, David Holmes wrote:
> 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
>>>>>>>>>
>>>>>>>>>
>>>>>
>>>>>
>>>
>>>
--
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the source code.
More information about the hotspot-runtime-dev
mailing list