Staffan, OK.
-Dmitry On 2012-05-04 14:11, Staffan Larsen wrote: > Note that there is _lots_ of old cruft in os_bsd.cpp that are leftovers from > the bsd port and needs to be cleaned up. > Basically the file is a copy of os_linux.cpp with things removed by #ifdefs. > > I don't think it is in the scope of Nils' current work to do this cleanup. > > /Staffan > > On 4 maj 2012, at 11:41, Dmitry Samersoff wrote: > >> 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 ... > -- Dmitry Samersoff Java Hotspot development team, SPB04 * There will come soft rains ...
