Page MenuHomePhabricator

[sanitizer] Support padding with spaces in Printf.
ClosedPublic

Authored by earthdok on Jun 27 2013, 7:22 AM.

Details

Reviewers
glider
samsonov

Diff Detail

Event Timeline

samsonov added inline comments.Jun 27 2013, 7:49 AM
lib/sanitizer_common/sanitizer_printf.cc
43

I would rename "num" to smth. like "absolute_value"

69–77

RAW_CHECK(pos > 0);

78

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?

95

Does negation works properly for INT_MIN here?

lib/sanitizer_common/tests/sanitizer_printf_test.cc
107

And now the function name doesn't make any sense. Please either rename it, or use different helper in Padding test.

145

Add tests for both 2-digit and 4-digit numbers with %3d / %03d.

earthdok updated this revision to Unknown Object (????).Jun 27 2013, 8:19 AM
  • addressed samsonov's comments
lib/sanitizer_common/sanitizer_printf.cc
43

done

69–77

done

78

I guess so.

95

Yeah, we have tests for that. It's just 2's complement.

lib/sanitizer_common/tests/sanitizer_printf_test.cc
107

done

145

done

samsonov accepted this revision.Jun 27 2013, 8:27 AM

LGTM

lib/sanitizer_common/sanitizer_printf.cc
71

I'd prefer
for (;pos >= 0 && num_buffer[pos] == 0; pos--) { ... }
here and below.

earthdok closed this revision.Dec 5 2014, 9:43 AM