AcademySoftwareFoundation / OpenColorIO

A color management framework for visual effects and animation.
https://opencolorio.org
BSD 3-Clause "New" or "Revised" License
1.76k stars 434 forks source link

Issue #1874 Cast to unsigned char for isspace. #1888

Closed pennelee closed 9 months ago

pennelee commented 10 months ago

Fix for: https://github.com/AcademySoftwareFoundation/OpenColorIO/issues/1874

Cast to unsigned char before using std:isspace().

From cppreference:

Like all other functions from cctype, the behavior of std::isspace is undefined if the argument's value is neither representable as unsigned char nor equal to EOF. To use these functions safely with plain chars (or signed chars), the argument should first be converted to unsigned char

linux-foundation-easycla[bot] commented 10 months ago

CLA Signed

The committers listed above are authorized under a signed CLA.

doug-walker commented 10 months ago

Thanks for this fix @pennelee. Please let us know if you have problems getting the CLA signed.

Would you please add a unit test along the lines of what Remi suggested?

pennelee commented 10 months ago

Hi @doug-walker , Yes, Sean is working on the CLA for the company, hope it will be sorted out soon.

Sure I can add the unit test Remi suggested, I had it locally but didn't push the change.

pennelee commented 10 months ago

Hi @doug-walker, I have added the unit test and also CLA is now signed, and I am approved as contributor.

doug-walker commented 10 months ago

Thanks for getting the CLA signed @pennelee. Could you add the comment I suggested to the unit test please?

pennelee commented 10 months ago

I'm sorry @doug-walker , which comment do you refer to?

doug-walker commented 10 months ago

Apologies @pennelee, I had made a comment but it had not been submitted. You should see it now I think.

pennelee commented 10 months ago

@doug-walker Thanks Doug I have updated with the unit test comment.

pennelee commented 10 months ago

Hi @remia , Could you please help to review this PR when you have time? Thank you!