On Tue, 15 Nov 2022 07:04:38 GMT, Kim Barrett <kbarr...@openjdk.org> wrote:
>> Xue-Lei Andrew Fan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> delete swp file
>
> src/hotspot/cpu/aarch64/vm_version_aarch64.cpp line 226:
>
>> 224: char buf[512];
>> 225: os::snprintf(buf, sizeof(buf), "0x%02x:0x%x:0x%03x:%d", _cpu,
>> _variant, _model, _revision);
>> 226: if (_model2) os::snprintf(buf+strlen(buf), sizeof(buf) - strlen(buf),
>> "(0x%03x)", _model2);
>
> Instead of using `strlen(buf)` (now called twice!) to get the number of
> characters written, use the result of the first call to `os::snprintf`.
Good point! Updated in the new commit.
> src/hotspot/os/bsd/attachListener_bsd.cpp line 251:
>
>> 249: BsdAttachOperation* BsdAttachListener::read_request(int s) {
>> 250: char ver_str[8];
>> 251: os::snprintf(ver_str, sizeof(ver_str), "%d", ATTACH_PROTOCOL_VER);
>
> We later use `strlen(ver_str)` where we could instead use the result of
> `os::snprintf`.
I think it is safe to use the result of `os::snprintf` for the computation of
max_len of the buf, isn't it?
- const int max_len = (sizeof(ver_str) + 1) +
(AttachOperation::name_length_max + 1) +
+ const int max_len = (ver_str_len + 1) + (AttachOperation::name_length_max +
1) +
AttachOperation::arg_count_max*(AttachOperation::arg_length_max + 1);
> src/hotspot/os/bsd/attachListener_bsd.cpp line 414:
>
>> 412: // write operation result
>> 413: char msg[32];
>> 414: os::snprintf(msg, sizeof(msg), "%d\n", result);
>
> Rather than using strlen(msg) in the next line, use the result from
> os::snprintf.
Updated to use the result from os::snprtinf in the new commit.
> src/hotspot/share/classfile/javaClasses.cpp line 2532:
>
>> 2530: // Print module information
>> 2531: if (module_name != NULL) {
>> 2532: buf_off = (int)strlen(buf);
>
> `buf_off` could be the result of `os::snprintf` instead of calling `strlen`.
Updated to use the result of `os::snprintf` in the new commit.
> src/hotspot/share/code/dependencies.cpp line 780:
>
>> 778: }
>> 779: } else {
>> 780: char xn[12]; os::snprintf(xn, sizeof(xn), "x%d", j);
>
> Pre-existing very unusual formatting; put a line break between the statements.
Yes. Updated in the new commit.
-------------
PR: https://git.openjdk.org/jdk/pull/11115
