[PATCH] [sanitizer] Improve scanf interceptor

Alexey Samsonov samsonov at google.com
Fri Feb 8 02:31:45 PST 2013



================
Comment at: sanitizer_common_interceptors_scanf.inc:85
@@ +84,3 @@
+    // Length modifier.
+    if (*p == 'j' || *p == 'z' || *p == 't' || *p == 'L' || *p == 'q') {
+      dir->lengthModifier[0] = *p;
----------------
Evgeniy Stepanov wrote:
> Alexey Samsonov wrote:
> > Can you use smth like internal_strchr(*p, "jztLq") here and below?
> It would be slow.
I don't think that may be critical, but will improve readability.

================
Comment at: sanitizer_common_interceptors_scanf.inc:152
@@ +151,3 @@
+// or 0 if the conversion is invalid.
+static int scanf_get_char_size(char l, char c) {
+  if (c == 'C' || c == 'S') {
----------------
"l" and "c" are probably too short names. Why not pass ptr to scanfDirective here?

================
Comment at: sanitizer_common_interceptors_scanf.inc:172
@@ +171,3 @@
+  if (scanf_is_integer_conv(dir->convSpecifier)) {
+    switch (dir->lengthModifier[0]) {
+    case 'h':
----------------
Evgeniy Stepanov wrote:
> Alexey Samsonov wrote:
> > Why did you drop arrays of pairs { ('j', sizeof(INTMAX_T) , ... }?
> This data does not tabulate well. We would have a special case for "hh" and "ll". And, anyway, it is used in one place only.
Okay. But probably you can make code somewhat denser
case 'j': return sizeof(INTMAX_T);
case 'h': return (dir->lengthModifier[1] == 'h') ? sizeof(char) : sizeof(short);
etc

================
Comment at: sanitizer_common_interceptors_scanf.inc:237
@@ +236,3 @@
+
+  if (dir->convSpecifier == 'c' || dir->convSpecifier == 'C') {
+    unsigned charSize = scanf_get_char_size(dir->lengthModifier[0],
----------------
This block can be merged with if () {...} for convSpecifiers {s,S,[} above


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



More information about the llvm-commits mailing list