baloghadamsoftware marked 5 inline comments as done.
baloghadamsoftware added inline comments.
================
Comment at: clang-tidy/misc/MisplacedOperatorInStrlenInAllocCheck.cpp:30
+ Finder->addMatcher(
+ callExpr(callee(functionDecl(hasName("malloc"))),
+ hasArgument(0, anyOf(hasDescendant(BadUse), BadUse)))
----------------
xazax.hun wrote:
> Maybe it is worth to have a configurable list of allocation functions?
>
> Maybe it would be worth to support`alloca` as well?
Support for ``alloca`` added.
I agree, it is worth to have a configurable list, but I think it is better to
be done in a separate patch.
================
Comment at: clang-tidy/misc/MisplacedOperatorInStrlenInAllocCheck.cpp:44
+ const MatchFinder::MatchResult &Result) {
+ // FIXME: Add callback implementation.
+ const auto *Alloc = Result.Nodes.getNodeAs<CallExpr>("Alloc");
----------------
xazax.hun wrote:
> What is this fixme?
Sorry, I forgot to remove it.
================
Comment at: clang-tidy/misc/MisplacedOperatorInStrlenInAllocCheck.h:19
+
+/// FIXME: Write a short description.
+///
----------------
xazax.hun wrote:
> There is a missing description.
No longer. Sorry, I forgot it.
================
Comment at: docs/clang-tidy/checks/list.rst:10
android-cloexec-creat
+ android-cloexec-dup
android-cloexec-epoll-create
----------------
Eugene.Zelenko wrote:
> I think will be better to fix order of other checks separately from this
> check.
I agree, but I did not do it. It was the script that created the new check. I
changed it back to the wrong order.
================
Comment at:
docs/clang-tidy/checks/misc-misplaced-operator-in-strlen-in-alloc.rst:16
+ void bad_malloc(char *str) {
+ char *c = (char*) malloc(strlen(str + 1));
+ }
----------------
aaron.ballman wrote:
> xazax.hun wrote:
> > What if this code is intentional for some reason?
> > I think in thase case it could be rewritten like the following (which is
> > much cleaner):
> > ```
> > char *c = (char*) malloc(strlen(str)-1);
> > ```
> > I think it might be a good idea to mention this possibility as a way to
> > suppress this warning in the documentation.
> This is my concern as well -- I'd like to know how chatty this diagnostic is
> on real-world code bases, especially ones that rely on C rather than C++. A
> somewhat common code pattern in Win32 coding are double-null-terminated lists
> of strings, where you have null terminated strings at adjacent memory
> locations with two null characters for the end of the list. This could result
> in reasonable code like `malloc(strlen(str + offset) + 1)`.
It is now more conservative: only ``+ 1`` counts, and only if there is no
additional ``+ 1`` outside.
https://reviews.llvm.org/D39121
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
