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

[Michael Jones via Phabricator via libc-commits]

michaelrj added inline comments.


================
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';
+
----------------
lntue wrote:
> 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.
In this case, I'd say keeping them separate makes it more clear that we're writing to `buff_cur - 1` and not `bufcur`, so I'm going to leave it.


================
Comment at: libc/src/stdio/printf_core/oct_converter.h:90
+
+  if (spaces < 0)
+    spaces = 0;
----------------
lntue wrote:
> 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.
spaces doesn't need to be reset, but zeroes does when it's being used in later calculations (see lines 73-76).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127985