[PATCH] [clang-tidy] Add a checker for zero-length memset.

Daniel Jasper djasper at google.com
Wed Jul 16 06:39:21 PDT 2014


================
Comment at: clang-tidy/google/MemsetZeroLengthCheck.cpp:32-33
@@ +31,4 @@
+                              hasArgument(2, integerLiteral(equals(0))),
+                              unless(hasArgument(1, integerLiteral())),
+                              unless(hasArgument(1, characterLiteral())),
+                              unless(InTemplateInstantiation)).bind("decl"),
----------------
Intuitively, I would have written something like:

  unless(hasArgument(1, expr(anyOf(integerLiteral(), characterLiteral())))

Also, I wonder whether we should do this in the callback or inside the callback. In the callback, we could check whether the argument trivially computes to 0 and thereby not only find literal zeros. I am thinking about cases like:

  int default_value = 0;
  memset(a, 10, default_value);

This is merely a suggestion, though, not sure whether it would lead to false positives.

================
Comment at: clang-tidy/google/MemsetZeroLengthCheck.cpp:65
@@ +64,3 @@
+  SourceRange LHSRange = Arg1->getSourceRange(),
+              RHSRange = Arg2->getSourceRange();
+  StringRef RHSString = getAsString(Result, RHSRange);
----------------
I generally dislike declaration statements defining more than one variable. Here, at lest make it consisten with RHSString and LHSString.

================
Comment at: clang-tidy/google/MemsetZeroLengthCheck.cpp:68
@@ +67,3 @@
+  StringRef LHSString = getAsString(Result, LHSRange);
+  if (!LHSString.empty() && !RHSString.empty()) {
+    D << FixItHint::CreateReplacement(CharSourceRange::getTokenRange(LHSRange),
----------------
Early exit?

  if (LHSString.empty() || RHSString.empty())
    return;

================
Comment at: clang-tidy/google/MemsetZeroLengthCheck.h:21-22
@@ +20,4 @@
+///
+/// This is most likely unintended and the swapped the length and value
+/// arguments.
+///
----------------
The sentence doesn't really parse. How about:

This is most likely unintended and the length and value arguments are swapped.

http://reviews.llvm.org/D4535






More information about the cfe-commits mailing list