RFR: 8327410: Add hostname option for UL file names [v3]

Fredrik Bredberg fbredberg at openjdk.org
Tue Mar 12 07:18:13 UTC 2024


On Tue, 12 Mar 2024 05:51:20 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Fredrik Bredberg has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Update after review
>
> src/hotspot/share/logging/logFileOutput.cpp line 400:
> 
>> 398:   if (pid != nullptr) {
>> 399:     result_len -= strlen(PidFilenamePlaceholder);
>> 400:     result_len += strlen(pid_string);
> 
> We should cache the `strlen` values here and elsewhere so that we don't have to keep recalculating it when doing the actual `strcmp` and `strcpy` later on.

I thought about that, but since this function is only called once at startup I didn't think it needed the optimization and added complexity. But if you think otherwise, I'll fix it.

> src/hotspot/share/logging/logFileOutput.cpp line 408:
> 
>> 406:   if (hostname != nullptr) {
>> 407:     if (!os::get_host_name(hostname_string, sizeof(hostname_string))) {
>> 408:       int res = jio_snprintf(hostname_string, sizeof(hostname_string), "%s", HostnameFilenamePlaceholder);
> 
> This seems overkill just to write `%hn` into `hostname_string`. And on error shouldn't we just replace the `%hn` with e.g. `unknown-host`?

I'll change to `unknown-host`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18153#discussion_r1520959352
PR Review Comment: https://git.openjdk.org/jdk/pull/18153#discussion_r1520957523


More information about the hotspot-runtime-dev mailing list