[libc-commits] [PATCH] D127985: [libc] add printf oct conversion

[Tue Ly via Phabricator via libc-commits]

lntue accepted this revision.
lntue added inline comments.
This revision is now accepted and ready to land.


================
Comment at: libc/src/stdio/printf_core/oct_converter.h:26
+  // ceil(number of bits / 3).
+  static constexpr size_t BUFF_LEN = ((sizeof(uintmax_t) * 8) + 2) / 3;
+  uintmax_t num = to_conv.conv_val_raw;
----------------
`static` is not needed here.


================
Comment at: libc/src/stdio/printf_core/oct_converter.h:37
+  for (; num > 0 /* && buff_cur > 0 */; --buff_cur, num /= 8)
+    buffer[buff_cur - 1] = (num % 8) + '0';
+
----------------
Optional: technically you can use `buffer[--buff_cur] = ...` and remove the `--buff_cur` in the line above.  But I'm not sure if it makes this less readable.


================
Comment at: libc/src/stdio/printf_core/oct_converter.h:90
+
+  if (spaces < 0)
+    spaces = 0;
----------------
Look like you don't need to reset spaces and zeros to 0 when they are negatives here and above, since you already check to make sure they are positive before using in the if-else's below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127985