[PATCH] [sanitizer] Support padding with spaces in Printf.

Alexey Samsonov samsonov at google.com
Thu Jun 27 07:49:19 PDT 2013



================
Comment at: lib/sanitizer_common/tests/sanitizer_printf_test.cc:107
@@ -106,3 +106,3 @@
 template<typename T>
 static void TestMinMax(const char *fmt, T min, T max) {
   char buf[1024];
----------------
And now the function name doesn't make any sense. Please either rename it, or use different helper in Padding test.

================
Comment at: lib/sanitizer_common/sanitizer_printf.cc:99
@@ +98,3 @@
+  bool negative = (num < 0);
+  return AppendNumber(buff, buff_end, (u64)(negative ? -num : num), 10,
+                      minimal_num_length, pad_with_zero, negative);
----------------
Does negation works properly for INT_MIN here?

================
Comment at: lib/sanitizer_common/sanitizer_printf.cc:43
@@ +42,3 @@
+// on the value of |pad_with_zero|.
+static int AppendNumber(char **buff, const char *buff_end, u64 num, u8 base,
+                        u8 minimal_num_length, bool pad_with_zero,
----------------
I would rename "num" to smth. like "absolute_value"

================
Comment at: lib/sanitizer_common/tests/sanitizer_printf_test.cc:131
@@ +130,3 @@
+  TestMinMax<int>("%03d - %03d", -1, 100);
+  TestMinMax<int>("%03d - %03d", -1, -100);
+}
----------------
Add tests for both 2-digit and 4-digit numbers with %3d / %03d.

================
Comment at: lib/sanitizer_common/sanitizer_printf.cc:70
@@ -59,3 +69,3 @@
   }
-  int result = 0;
-  while (pos-- > 0) {
+  pos--;
+  while (pos > 0 && num_buffer[pos] == 0 && num_buffer[pos-1] == 0) {
----------------
RAW_CHECK(pos > 0);

================
Comment at: lib/sanitizer_common/sanitizer_printf.cc:79
@@ +78,3 @@
+    pos--;
+  }
+  if (negative && !pad_with_zero) result += AppendChar(buff, buff_end, '-');
----------------
Oh, this is ugly. Can you write
while (pos >= 0 && num_buffer[pos] == 0) {
  char c = (pad_with_zero || pos == 0) ? '0' : ' ');
  result += AppendChar(buff, buff_end, c);
  pos--;
}
instead of while + if?


http://llvm-reviews.chandlerc.com/D1057



More information about the llvm-commits mailing list