mkindahl wrote: > Overall I'm a bit hesitant to take this check. > > > I ran a quick test (with the check this PR is adding) on a few repos I had > > locally, with the following result: > > These numbers show that calling `setbuf`/`setvbuf` with a stack-allocated > buffer _at all_ is rare. But what we want to know is, of the cases where it > _is_ done, is a significant number of them a bug? If calls with stack buffers > are rare but almost always correct, that doesn't justify always warning on > them.
I agree with this reasoning, which is why I started looking for issues at GitHub that contained fixes related to setbuf/setvbuf and those fixes I found seemed to move from using a stack-allocated to using malloc-like functionality: that is, this check would have caught those problems and not warned after the fix. I have not yet found any stack-allocated buffers where fclose is called, but I'll do a more through research. > Without that kind of evidence, I don't think we should take this check > without logic to detect when calls like `fclose` or a subsequent call to > `setbuf(..., NULL)` make the stack buffer usage safe, but since that kind of > detection is necessarily flow-control based, that leads me to think this > check would be a better fit for the Clang Static Analyzer. I looked for setbuf/setvbuf cases similar to what you proposed, but could not find any. It is clear that using a Static Analyzer would capture more errors, but I see a value in capturing the most common problems if they can be caught without triggering false positives. If there is a lot of false positives, it diminishes the value of the check. My reasoning is this: - The most common problem is stack-allocating a buffer and not using fclose(). Avoiding this problem is a net-benefit. - The case where you create a local buffer and then close it is close to non-existent. Looking through issues and code on GitHub provides a reasonable indication if these are true or not, so I'll do that and get back. https://github.com/llvm/llvm-project/pull/187637 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
