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