RFR(M) 8193373: Cleanup ElfFile and family

Zhengyu Gu zgu at redhat.com
Tue Feb 13 18:00:53 UTC 2018


Thanks, Yumin.

-Zhengyu

On 02/13/2018 12:47 PM, yumin qi wrote:
> Looks good to me!
> 
> Thanks
> Yumin
> 
> On Mon, Feb 12, 2018 at 6:44 PM, Zhengyu Gu <zgu at redhat.com 
> <mailto:zgu at redhat.com>> wrote:
> 
>     Hi Yumin,
> 
>     I made change according to your comment. Also, added a test.
> 
>     Webrev: http://cr.openjdk.java.net/~zgu/8193373/webrev.01/index.html
>     <http://cr.openjdk.java.net/~zgu/8193373/webrev.01/index.html>
> 
>     Test:
>        Reran hotspot_runtime tests on Linux 64 (fastdebug and release)
> 
>     Thanks,
> 
>     -Zhengyu
> 
> 
> 
>     On 02/08/2018 12:51 PM, yumin qi wrote:
> 
>         Hi, Zhengyu
> 
>             Looks good to me. One minor comment:
> 
>         +bool ElfFile::specifies_noexecstack(const char* filepath) {
>         + // Returns true if the elf file is marked NOT to require an
>         executable stack,
>         + // or if the file could not be opened.
>         + // Returns false if the elf file requires an executable stack,
>         the stack flag
>         + // is not set at all, or if the file can not be read.
> 
> 
>             should the comments in side the function be moved above the
>         function?
> 
>              others looks OK to me.
> 
>         Thanks
>         Yumin
> 
> 
> 
> 
>         On Mon, Feb 5, 2018 at 8:08 AM, Zhengyu Gu <zgu at redhat.com
>         <mailto:zgu at redhat.com> <mailto:zgu at redhat.com
>         <mailto:zgu at redhat.com>>> wrote:
> 
>              Please review this cleanup and refactoring of elf decoder.
> 
>              * Fixed coding styles.
>                 - Moved non-static members to the top of classes
>                 - Fixed variable names (from Windows naming style to
>         hotspot style)
> 
>              * Moved Linux implementation of
>         ElfFile::specifies_noexecstack to os
>                 directory
> 
>              * Refactored section cache logic into common ElfSection class.
> 
>              * Added utility classes FileReader and MarkedFileReader to
>         cleanup
>                 duplicated code.
> 
>              * Deleted section header string table once initialization
>         is completed,
>                 cause it is not used for decoding.
> 
>              * When decoder failed to cache section data, current
>         implementation
>                 tries to fallback to read data from file directly. This
>         never
>                 worked, cause it put decoder into out of memory state
>         when it failed
>                 load section data, so decoder always bailed out prematurely.
>                 This patch also fixed this bug.
> 
>              Bug: https://bugs.openjdk.java.net/browse/JDK-8193373
>         <https://bugs.openjdk.java.net/browse/JDK-8193373>
>              <https://bugs.openjdk.java.net/browse/JDK-8193373
>         <https://bugs.openjdk.java.net/browse/JDK-8193373>>
>              Webrev: http://cr.openjdk.java.net/~zgu/8193373/webrev.00/
>         <http://cr.openjdk.java.net/~zgu/8193373/webrev.00/>
>              <http://cr.openjdk.java.net/~zgu/8193373/webrev.00/
>         <http://cr.openjdk.java.net/~zgu/8193373/webrev.00/>>
> 
> 
>              Test:
> 
>                 hotspot_runtime on Linux 64 (fastdebug and release)
> 
>              Thanks,
> 
>              -Zhengyu
> 
> 
> 


More information about the hotspot-runtime-dev mailing list