Nils,
os_bsd.cpp:
2359: // Print warning if unsafe chroot environment detected
constant below is
unstable_chroot_error
2373: Never heard about neither about /etc/XXX-release on BSD nor
about mandrake or redhat bsd.
Please fix it as well.
2409: st->print(os::Bsd::glibc_version())
glibc_version() looks a little bit weird as
none of BSD has glibc.
-Dmitry
On 2012-05-04 12:26, Nils Loodin wrote:
> Updated this with pulling out some shared code to os_posix.cpp.
> Some methods were different enough that this wasn't possible though.
>
> Do you like this better?
>
> Webrev:
> http://cr.openjdk.java.net/~nloodin/7165755/webrev.00/
>
> On May 2, 2012, at 14:08 , Nils Loodin wrote:
>
>>
>> On May 2, 2012, at 14:00 , David Holmes wrote:
>>
>>> Hi Nils,
>>>
>>> On 2/05/2012 9:56 PM, Nils Loodin wrote:
>>>>> Ignoring Windows (with prints very little) I'd say it is the printing of
>>>>> /proc/meminfo that is the main difference. Not sure why printing that was
>>>>> necessary ... but if we are going to remove it I think we need to know
>>>>> why it was added.
>>>> Yes, that's the reason.
>>>> Note that nothing is removed. The method still prints exactly the same
>>>> info, but I introduced another method to print briefer info, to be kinder
>>>> to tool developers.
>>>
>>> The current one prints /proc/meminfo. You turned that code into
>>> print_full_memory_info but in the main routine you call print_memory_info.
>>> Was that a mistake?
>>
>> YES! Glad you caught that :) Guess (or hope) it would have been caught in
>> dump testing otherwise :)
>>
>> Regards,
>> Nils Loodin
>>
>>
>>>
>>> David
>>>
>>>> I really don't want to change the output for say, hs_err files, where I
>>>> believe this info is used.
>>>>
>>>>>
>>>>>> This can make it hard for tool writers to get a summary that look good
>>>>>> and similar for multiple platforms (sizing of gui fields, having to
>>>>>> parse info in the tool code etc)
>>>>>> Lookin at the code, it's in some serious need of refactoring. It would
>>>>>> be nice with a method to get a "brief" os info for these kinds of tools
>>>>>> that looks similar on all platforms.
>>>>>>
>>>>>> This is my suggested change:
>>>>>> http://cr.openjdk.java.net/~nloodin/7165755/webrev.00/
>>>>>
>>>>> Seems to me some of this could be factored into the top-level OS class if
>>>>> we shoehorn Windows into the same shape as the other OSes ;-)
>>>> This was my first attempt also, but then a lot of empty windows-methods
>>>> ensued, which was kind of ugly.
>>>>
>>>>> Or at least perhaps put some of the common stuff into os_posix.cpp ?
>>>> There's a thought!
>>>> I'll investigate that route, it could get things to look nicer.
>>>>
>>>>
>>>>> Cheers,
>>>>> David
>>>> Regards,
>>>> Nils Loodin
>>>>
>>
>
--
Dmitry Samersoff
Java Hotspot development team, SPB04
* There will come soft rains ...
