RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]

[suchismith1993]

On Thu, 23 Nov 2023 06:03:15 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> suchismith1993 has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   change macro position
>
> src/hotspot/os/aix/os_aix.cpp line 3065:
> 
>> 3063: //Replaces provided path with alternate path for the given file,if it doesnt exist.
>> 3064: //For AIX,this replaces .so with .a.
>> 3065: void os::Aix::mapAlternateName(char* buffer, const char *extension) {
> 
> The documentation is wrong: 
> 
> // Replaces the specified path with an alternative path for the 
>    given file if the original path doesn't exist
> 
> It does no such thing; it replaces the extension unconditionally. The comment sounds like it does a file system check.
> 
> The whole function is not that well named - "map alternate name" does not really tell me anything, I need to look at the implementation and the caller to understand what it is doing. There is no mapping here, this is just a string utility function.
> 
> The function should not modify the original buffer but instead assemble a copy. That is the conventional way to do these things. You can work with immutable strings as input, e.g. literals, and don't risk buffer overflows.
> 
> All of this should be handled inside os_aix.cpp; see my other comment. This should not live in the external os::aix interface, since it has nothing to do with AIX. But I think all of this should be confined to os_aix.cpp.
> 
> Proposal for a clearer name, comment, and pseudocode
> 
> // Given a filename with an extension, return a new string containing the filename with the new extension.
> // New string is allocated in resource area.
> static char* replace_extension_in_filename(const char* filename, const char* new_extension) {
>   - allocate buffer in RA
>   - assemble new path by contacting old path - old extension + new extension
>   - return new path
> }

thank you for the explanation. I am working on it.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16604#discussion_r1404265530