RFR(M): 8206977: Minor improvements of runtime code.

[coleen.phillimore at oracle.com]

http://cr.openjdk.java.net/~goetz/wr18/8206977-covRuntime/01/src/hotspot/share/classfile/moduleEntry.cpp.udiff.html

+ name ? name->as_C_string() : "<unnamed>");


Can you change to:

+ name != NULL ? name->as_C_string() : "<unnamed>");


http://cr.openjdk.java.net/~goetz/wr18/8206977-covRuntime/01/src/hotspot/share/oops/klassVtable.cpp.udiff.html

This looks a lot nicer!   Similar code is in linkResolver.cpp, can you 
look at changing it too?

http://cr.openjdk.java.net/~goetz/wr18/8206977-covRuntime/01/src/hotspot/share/services/writeableFlags.cpp.udiff.html

If name is null here, what would this do?  Should there be an 'else' to 
print something?

I think this looks fine.  It doesn't look major to me.  The asserts 
turned to guarantees don't appear to be anywhere performance sensitive.

Thanks,
Coleen



On 7/10/18 6:53 AM, Lindenmaier, Goetz wrote:
> Hi,
>
> I ran coverity on the jdk11 hotspot sources and want to propose the
> following fixes to the runtime code. I scanned the linux x86_64 build.
> Some issues are similar to previous parfait fixes (check for NULL, add
> guarantees etc.) I also identified some issues I consider real problems.
> http://cr.openjdk.java.net/~goetz/wr18/8206977-covRuntime/01/
>
> In detail:
>
> Real issues:
> ------------
>
> jvmtiEnvBase.cpp
>    || should be &&.
>    Attention, this is the only change that really will change behaviour.
>    But if thr == NULL we will see a crash below.
>
> perfMemory_linux.cpp:
>    Wrong buffer length used.
>
> systemDictionary.cpp:
>    Move code dereferencing ik under if (ik != NULL).
>
> virtualspace.cpp
>    Initialization is missing. Moved constructor up to the other
>    constructors.
>
>
> Useful code improvements:
> -------------------------
>
> vm_version_ext_x86.cpp
>    Assure buffer is not accessed at offset -1.
>
> os_linux.cpp
>    Numa_max_node returns int, and a -1 in some cases.
>
> moduleEntry.cpp
>    name might be NULL. Just a fix for tracing.
>
> systemDictionaryShared.cpp
>    clearify code.
>    It would be wrong if only entry == NULL would hold, one
>    would hit the assertion below.
>
> verifier.cpp
>    Fix tracing.
>    Illegal opcode is -1 and should not be passed to name array.
>
> logOutput.cpp
>    If n_selections == 0, best_selection would be NULL.
>    Move up the assertion and turn into a guarantee.
>
> filemap.cpp
>    Either base can be NULL, or parts of the code before are dead.
>
> metaspace.cpp
>    We now an exception is pending.
>
> klassVtable.cpp
>    Coverity does not like the format in a variable.
>    Anyways this is quite rough coding, transformed to use stringStream
>    as with other similar exceptions.
>
> jvmFlag.cpp
>    match might be NULL.
>
> writableFlags.cpp
>    name might be NULL.
>
> ostream.cpp
>    If ftell returns error code -1, we need not continue.
>    Especially we should not fseek(-1).
>
> logTestUtils.inline.hpp
>    ftell returns -1.
>
> test_metachunk.cpp
>    wrong datatype.