RFR: 8345012: os::build_agent_function_name potentially wastes a byte when allocating the buffer [v5]

[Thomas Stuefe]

On Fri, 29 Nov 2024 01:29:11 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> This is a minor issue noticed in the review of JEP 479. The calculations of the buffer length always do `+2` to allow for an optional underscore plus the terminator. A solution was suggested in that review (which I initially used) but I've instead opted for a solution where `name_len` always reflects the actual length of the lib name - otherwise we need to subtract the 1 again when doing the `strncat`. I also made the need for the underscore explicit. 
>> 
>> As the same code is used in os_posix.cpp both versions were made consistent. The only difference between them is the Windows version has to check for a drive specifier. I toyed with combining them into a shared version with a Windows-specific chunk, but opted for the simpler change. I can revisit that if people have strong opinions.
>> 
>> Testing
>> - tiers 1-4
>> 
>> Thanks
>
> David Holmes has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Extra blank line rmeoved

Okay. Minor nits, leave it up to you if you fix them:
- I would consider renaming name_len to libname_len, may have prevented my original confusion
- I would consider adding a test for prefix (libname must start with prefix) and possibly suffix. We handle this partly with `if (name_len <= (prefix_len + suffix_len)) { return null }`, so this seems to be something that can occur at runtime.
- OOM is still misreported as "cannot find symbol"

-------------

Marked as reviewed by stuefe (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22404#pullrequestreview-2469227989