On Thu, 8 Dec 2022 17:19:56 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:
>>
>> update on review feedback
>
> src/hotspot/share/adlc/formssel.cpp line 28:
>
>> 26: #include "adlc.hpp"
>> 27:
>> 28: #define remaining_buflen(buffer, position) (sizeof(buffer) - (position -
>> buffer))
>
> Consider additional parenthesis, e.g. `((position) - (buffer))`.
It makes sense to me. Thanks!
> src/hotspot/share/adlc/formssel.cpp line 1538:
>
>> 1536: }
>> 1537: // Add predicate to working buffer
>> 1538: snprintf_checked(s, remaining_buflen(buf, s),
>> "/*%s*/(",(char*)i._key);
>
> [pre-existing] the result from this snprintf could be used instead of strlen
> in the next line.
I'm not very sure of why "+=" is used yet. I would like to leave as it is.
> src/hotspot/share/adlc/output_c.cpp line 546:
>
>> 544: char* args = new char [36 + 1];
>> 545:
>> 546: snprintf_checked(args, 37, "0x%x, 0x%x, %u",
>
> Instead of 37, consider `36 + 1` to be clearly from the line above. Or give
> that value a name.
Updated to use 36 + 1.
-------------
PR: https://git.openjdk.org/jdk/pull/11115
