URG! Need a second reviewer! Re: RR(S): JDK-8025812 tmtools/jmap/heap_config tests fail on Linux-ia32 because it 'Can't attach to the core file'

[David Holmes]

Hi Dmitry,

In

  716 static inline size_t round_page(int x){
  717   static int page_size;
  718
  719   page_size = sysconf(_SC_PAGE_SIZE);
  720   return ROUNDUP(x,page_size);
  721 }

Does page_size being static mean that line 719 will only be executed 
once? Otherwise it serves no purpose.

Typo: exisiting

Otherwise seems okay.

Thanks,
David

On 17/10/2013 7:11 AM, Dmitry Samersoff wrote:
> On 2013-10-14 11:49, Staffan Larsen wrote:
>> The fix looks good, but I have a problem with the ROUNDUP_PAGE macro. First, I don't like having macros defined in the middle of a method. Second, the definition of the macro includes the value of a local variable which is a bit hairy. Can't you just ROUNDUP directly in the four places it's needed? I think it would make for more readable code.
>>
>> nit on line 743: filed -> field
>>
>> Thanks,
>> /Staffan
>>
>> On 12 okt 2013, at 13:25, Dmitry Samersoff <Dmitry.Samersoff at oracle.com> wrote:
>>
>>> Hi Everybody,
>>>
>>> Please review the fix
>>>
>>> http://cr.openjdk.java.net/~dsamersoff/JDK-8025812/webrev.01/
>>>
>>> The value of p_memsz filed of elf header of LOAD section inside coredump
>>> is rounded up to page size. So round up corresponding value read from
>>> the header of library it self.
>>>
>>> --
>>> Dmitry Samersoff
>>> Oracle Java development team, Saint Petersburg, Russia
>>> * I would love to change the world, but they won't give me the sources.
>>
>
>