google / emboss

Emboss is a tool for generating code that reads and writes binary data structures.
Apache License 2.0
68 stars 21 forks source link

runtime: TextOutputOptions::WithNumericBase should take a uint8_t #56

Closed fsareshwala closed 1 year ago

fsareshwala commented 1 year ago

The TextOutputOptions::numeric_base_ field is stored as a uint8_t. However, TextOutputOptions::WithNumericBase takes an int parameter. Albeit unlikely, this allows for some potential data loss since an int on most systems is four bytes whereas a uint8_t is only a single byte.

A previous commit (85f3b790) used static_cast to remove the -Wimplicit-int-conversion compiler warning. However, we don't actually need a static cast here. Instead, we should just update the parameter type to be uint8_t since that's the type used in internal storage as well. It's unlikely that anyone will want a numeric base larger than 255 anyway, but if they do, they should deal with that at the call site rather than Emboss deciding to eliminate data under the hood.

This change updates TextOutputOptions::WithNumericBase to take a uint8_t in as its parameter instead of an int.