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