JDK-8259843: initialize dli_fname array before calling dll_address_to_library_name

[Schmidt, Lutz]

Hi David, 
my comment was motivated by the "don't fail if you don't have to" principle. You will certainly agree, such principle is very helpful in productive environments. In this particular case, there is no need to fail - just don't print. On the other hand, if these checks describe a "cannot occur situation" - why then checking for it at all?

Anyway, life has gone on since I wrote my comment. Everybody else likes the path you (and Thomas) suggested. I will therefore mute myself. My LGTM statement remains valid. 

Best Regards,
Lutz

On 20.01.21, 13:20, "David Holmes" <david.holmes at oracle.com> wrote:

    On 20/01/2021 6:32 pm, Lutz Schmidt wrote:
    > On Wed, 20 Jan 2021 08:21:36 GMT, Matthias Baesken <mbaesken at openjdk.org> wrote:
    > 
    >>> src/hotspot/share/runtime/frame.cpp line 543:
    >>>
    >>>> 541:   bool found;
    >>>> 542:
    >>>> 543:   if (buf == NULL || buflen < 1) return;
    >>>
    >>> Can this not just be an assert: buf != NULL && buflen > 0 ?
    >>
    >> Hi David,  I think a return would be clearer but an assert is "better than nothing" .
    >>
    >> Best regards, Matthias
    > 
    > With an assert, you assume this is a "cannot occur error". You should be pretty sure to have good test coverage to find all "illegal invocations" before letting a release build escape into the wild.

    The problem with a return is that it implies these conditions are 
    allowable and could reasonably happen, when in fact it would be an 
    internal programming error in the VM. Do we have test coverage for all 
    the code paths involved? Probably not. But the vast majority of 
    assertions in the VM do not have a "release" bail-out path. Keeping one 
    here is unnecessary and overkill IMO but I won't insist it be removed as 
    this is essentially error reporting code anyway.

    David

    > -------------
    > 
    > PR: https://git.openjdk.java.net/jdk/pull/2144
    >