RFR: 8332400: isspace argument should be a valid unsigned char [v2]

Julian Waters jwaters at openjdk.org
Wed Jun 12 10:31:16 UTC 2024


On Tue, 11 Jun 2024 18:07:10 GMT, Robert Toyonaga <duke at openjdk.org> wrote:

>> ### Summary
>> This change ensures we don't get undefined behavior when calling[`isspace`](https://pubs.opengroup.org/onlinepubs/007904975/functions/isspace.html).  `isspace` accepts an `int` argument that "the application shall ensure is a character representable as an unsigned char or equal to the value of the macro EOF.". 
>> 
>> Previously, there was no checking of the values passed to `isspace`. I've replaced direct calls with a new wrapper `os::is_space` that performs a range check and prevents the possibility of undefined behavior from happening. For instances outside of Hotspot, I've added casts to `unsigned char`. 
>> 
>> **Testing**
>> - Added a new test in `test/hotspot/gtest/runtime/test_os.cpp` to check `os::is_space` is working correctly.
>> - tier1
>
> Robert Toyonaga has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Replace wrapper with casts.

Had I seen this earlier I would have suggested more C++ styled casts to make which conversion exactly is happening clearer, but oh well. I agree that it's unfortunate that this has to be done

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

Marked as reviewed by jwaters (Committer).

PR Review: https://git.openjdk.org/jdk/pull/19567#pullrequestreview-2112608501


More information about the serviceability-dev mailing list