[libc-commits] [PATCH] D125929: [libc] add printf base 10 integer conversion

[Michael Jones via Phabricator via libc-commits]

michaelrj added inline comments.


================
Comment at: libc/src/stdio/printf_core/int_converter.h:39
+  static constexpr size_t buff_len = digits_to_store<uintmax_t>();
+  uintmax_t num = to_conv.conv_val_raw;
+  char buffer[buff_len];
----------------
sivachandra wrote:
> Can this assignment ever lead to a truncation? May no because this is a `convert_int` which does not deal with integers wider than 64-bit?
It can't, because `uintmax_t` is at least as large as the largest valid integer type for printf. The size of `conv_val_raw` may be larger than `uintmax_t` to fit `long double` values, but the value passed by the user can't be larger than `uintmax_t`.


================
Comment at: libc/src/stdio/printf_core/int_converter.h:59
+  case LengthModifier::none:
+    num = num & type_mask<int>();
+    break;
----------------
sivachandra wrote:
> Why are these masking operations required? Also, for max values, we have: https://github.com/llvm/llvm-project/blob/main/libc/src/__support/CPP/Limits.h
This is to handle the fact that the length modifiers allow the user to specify the size of the integer to be converted. The type in `num` is always `uintmax_t` since that can fit any valid type, but the user can specify that they want it to be treated as a `short`, in which case we have to limit the value of `num` to what can fit in a short. The way this is done here is by creating a mask of just the bits in the specified type.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125929/new/

https://reviews.llvm.org/D125929