[PATCH] D35372: [clang-tidy] Add a close-on-exec check on memfd_create() in Android module.

Yan Wang via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 17 10:51:52 PDT 2017


yawanng added a comment.

In https://reviews.llvm.org/D35372#810525, @alexfh wrote:

> In https://reviews.llvm.org/D35372#810457, @alexfh wrote:
>
> > I have deja vu ;)
> >
> > Should we make a single check for all CLOEXEC and friends with a single configurable list of (function name, flag name) pairs?
>
>
> Okay, it may be a bit more complicated than just a list of function name -> flag name mappings, since we have to take in account the argument position as well. We also might want to check the signature to a certain degree to avoid matching wrong function. There are multiple approaches possible to rule out incorrect functions with the same name:
>
> 1. just look at the number of arguments - this might well be enough, since for a certain codebase I wouldn't expect multiple `memfd_create`'s etc. It would allow user configurability of the function -> flag mappings.
> 2. encode the types of arguments as strings and have a small dictionary of matchers in the check (e.g. `"const char*" -> pointerType(pointee(isAnyCharacter()))`) - that will be more precise and still quite flexible and also allow user-configurable function -> flag mappings. But this mechanism may be an overkill, if we don't anticipate user-configurable functions. I don't know how complex the resulting code turns out to be.
> 3. Add a matcher for each function statically. This would obviously allow for arbitrarily complex matchers, but won't be extensible via configuration options.


Great idea. But we may prefer separate checks, because each API can be controlled independently. Moreover, there are not that many such system functions and hopefully we have listed most of them already :-)  To reduce the code duplication, what about a base class for all of them and try to share as much code as possible?


https://reviews.llvm.org/D35372





More information about the cfe-commits mailing list