[clang] [clang][Analyzer] Move checker 'alpha.unix.Errno' to 'unix.Errno'. (PR #69469)

via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 15 07:29:08 PST 2023


https://github.com/DonatNagyE approved this pull request.

> > I also think that the [reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=postgres_REL_13_0_errno_alpha_unix&newcheck=postgres_REL_13_0_errno_unix&is-unique=on&diff-type=New&report-id=3253666&report-hash=b4e0b723164236269fe6078ad32a0456&report-filepath=%2apg_basebackup.c) [from](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=postgres_REL_13_0_errno_alpha_unix&newcheck=postgres_REL_13_0_errno_unix&is-unique=on&diff-type=New&report-id=3254052&report-hash=619eb5d998324adb5e02d9e3302bb4d5&report-filepath=%2acopy.c) [postgres](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=postgres_REL_13_0_errno_alpha_unix&newcheck=postgres_REL_13_0_errno_unix&is-unique=on&diff-type=New&report-id=3256373&report-hash=32f8e213c6fb419277ec76c40bfa3956&report-filepath=%2afe-connect.c) that you mentioned are too confusing.
> 
> After the change in #71392 the report looks like [this](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=postgres_REL_13_0_test_71392_base&newcheck=postgres_REL_13_0_test_71392&is-unique=on&diff-type=Unresolved&checker-name=alpha.unix.Errno&report-hash=0b2d4e61a48cd556bb982b39969b81d2&report-id=3386353&report-filepath=%2acopy.c). This looks somewhat better, probably still false positive because the "len" can not be 0, but the checker does not have more information.

Thanks for the update!

I think these improvements are sufficient to move this checker out of alpha.

The new message which clarifies that the report is produced in the case _"Assuming that argument 'size' to 'fwrite' is 0; 'errno' becomes undefined after the call"_ is completely sufficient to orient the user; and although the report is (arguably) a false positive, the user can quickly understand and discard it.

@steakhal or anybody else interested:
Is there any objection to merging #71392 (the commit that clarifies the note tags) and this commit?

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


More information about the cfe-commits mailing list