[clang-tools-extra] [clang-tidy] Add bugprone-unsafe-api-functions-calls (PR #187637)

[Mats Kindahl via cfe-commits]

mkindahl wrote:

> First off, by usage of "—" and comments in code it seems to be partially/all written by AI. Without notice about AI, it's violation of https://llvm.org/docs/AIToolPolicy.html: "Contributors are expected to be transparent and label contributions that contain substantial amounts of tool-generated content"

Yes, I am using Claude Code as part of development. I wasn't aware that you needed to add a trailer. Do you want it in the commit, the PR, or both?

> This check must have false positives when buffer is used safely (correct me if I'm wrong please)
> 
> ```c++
> void good_setvbuf(void) {
>     char buf[BUFSIZ];
>     setvbuf(stdout, buf, _IOFBF, BUFSIZ);
> 
>     // ...
> 
>     fflush(stdout);
>     setvbuf(stdout, nullptr, _IONBF, 0);
> }
> ```
> 
> So we should check for "safe usages" as well before emitting diagnostic.

This is a very rare case and if the developer really needs this they can use `NOLINT` to silence it. Most uses either heap-allocate a buffer or use a static buffer. I think it is more important to capture the idiomatic faulty cases. 

Also, if you require the capture of `fflush()`, what about other functions that call `fflush()`? For example, shall we capture `_fcloseall()`?

Looked for issue regarding `setvbuf` that actually sets a buffer. Most use NULL, but [this one](https://github.com/neovim/neovim/pull/26790/changes) uses a `malloc()`-allocated buffer (strictly speaking, `xmalloc`).

I ran a quick test on a few repos I had locally, with the following result:

| Codebase     | setvbuf/setbuf calls | Warnings |
|--------------|----------------------|----------|
| PostgreSQL   | 14                   | 0        |
| Apache httpd | 1                    | 0        |
| CPython      | 15                   | 0        |

Incidentally, neither use buffers (they just use NULL). It seems that is not a very common case.

> This check will currently also run for C++, when we can do:
> 
> ```c++
> struct StdoutBuffer {
>     char buf[BUFSIZ];
> 
>     StdoutBuffer()  { setbuf(stdout, buf); }
>     ~StdoutBuffer() { fflush(stdout); setbuf(stdout, nullptr); }
> };
> ```

It is an interesting case, but adding a `NOLINT` for these also makes sense since it is a very rare case.

https://github.com/llvm/llvm-project/pull/187637