[SERVER-69612] Audit the codebase for signed use of char types Created: 13/Sep/22 Updated: 27/Oct/23 Resolved: 15/Sep/22 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Task | Priority: | Major - P3 |
| Reporter: | Josef Ahmad | Assignee: | Jordi Olivares Provencio |
| Resolution: | Works as Designed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||
| Sprint: | Execution Team 2022-09-19 | ||||||||||||||||||||
| Participants: | |||||||||||||||||||||
| Description |
|
Signedness of "char" is platform-specific, e.g. it is signed on x86-linux, it is typically unsigned on ARM. For portability we should stick to using an unsigned type. |
| Comments |
| Comment by Jordi Olivares Provencio [ 15/Sep/22 ] |
|
In cases where char is used to contain a number it is always below 128, so uses of it are correct. The other uses for a char are to treat it as a byte (uses of which could be replaced by std::byte). geert.bosch@mongodb.com I haven't found a problem other than potential confusion by casting the char as a byte so I'll close the ticket. I've opened |
| Comment by Jordi Olivares Provencio [ 15/Sep/22 ] |
|
Looking at other branches with the same patterns leads to the same conclusion: Only the Enum handling worries me, but it seems to be correctly used as they are interpreted as a single raw byte when used. Something that could be improved is the single byte handling of the BSON BufBuilder. There's various methods used for handling a char but conceptually we're using them to append a single byte. Refactoring the code so that the 3 char methods are replaced by a method called appendByte(std::byte) would help clarify the intent. |
| Comment by Jordi Olivares Provencio [ 14/Sep/22 ] |
|
Searching for the following:
Points largely to the BSONType and BSONBinData enum handling. In particular two key values can be affected by the signedness of char: BSONType::MinKey and BSONBinData::bdtCustom as they are -1 and 128 respectively. |
| Comment by Gregory Noma [ 13/Sep/22 ] |
|
std::byte might be useful |