[PATCH] D71001: [clang-tidy] New check: bugprone-misplaced-pointer-arithmetic-in-alloc

Thu Dec 5 04:38:58 PST 2019

MyDeveloperDay added a comment.

Drive by comment: (I have no vested interested other than seeing cool new checkers)

FWIW, I work on a system with a multi million line legacy 25+ years old C++ codebase, where we now have clang-tidy integrated into a CI system and we have > 100,000 firings of clang-tidy warnings (with most non platform specific checks turned on),  these get collated into a database based on checker type so help us triage them.

I find myself focusing on those checks which have < 10 firings more than those that report 1000's, I like very specific checkers, unlike the checkers like `readability-convert-member-function-to-static` that tends to report hundreds of warnings but the false positive rate is very high. IMHO The more general the checker the more false positives there seems to be (just personal observation, not backed by data)

When I see new checkers being sent for review I like eyeball (normally with grep) some of my core libraries to see what chance could I be hit by the checker, in order to assess its usefulness (to me at least)

Just to give you an example, I pulled out 3 malloc's from one directory.

`::malloc(a + b+ sizeof(void *));`
`(malloc((a+ 1) * sizeof(char *)));`
`malloc(sizeof(a) + (unsigned)b)`

Now I don't think any of these mallocs would fire this checkers, but I think I'm just a fat finger away from it! Especially if this is the kind of code which exists elsewhere in my code base.

We all know writing code like this today isn't the best way, but clang-tidy is being used to check code which is hidden deep inside our repos, in places people fear to tread or rarely visit or more to the point are too scared to visit let alone touch.

I don't think we should measure the usefulness of a checker based just on it checking new modern c++ code.

I also don't think we should measure a checker by the number of firings it gives. It might only take one bug to cause a pacemaker to stop or a car to crash, or to let a hacker in.

If it can happen, it does happen or it has happened, I like the idea of having a checker to prevent it.

I'm not a fan of the "maintenance" is a burden argument to prevent new stuff coming in. No matter how true it might be. Maintenance of software has been my life for 30 years, in fact, I quite enjoy it. And aren't a lot of us here for fun and giving our timing freely to explicitly help with that burden?

As we just changed some of our communication systems to encourage new people in. Thats not going to work if we beat them back at the door with "its not coming in".

Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71001/new/

https://reviews.llvm.org/D71001