fixed some -Wsuggest-attribute=returns_nonnull GCC warnings

[firewave]

No description provided.

[firewave]

I manually checked all the function usages and in almost all cases we already used them like they were returning a non-null result.

[firewave]

I also filed llvm/llvm-project#106392 upstream about detecting the unnecessary checks.

[danmar]

I assume that clang/gcc will ensure that the functions do NOT return null if it's marked with that attribute? Otherwise removing the safety checks would look dangerous.

[danmar]

hmm it does not seem that clang/gcc warns properly when pointer could be NULL:

daniel@laptop:~/cppchecksolutions/cppcheck$ g++ -fsyntax-only -Wall -Wextra -pedantic 1.cpp
daniel@laptop:~/cppchecksolutions/cppcheck$ clang++ -fsyntax-only -Weverything 1.cpp
1.cpp:2:39: warning: no previous prototype for function 'p' [-Wmissing-prototypes]
__attribute__((returns_nonnull)) int* p(int*p) {
                                      ^
1.cpp:2:34: note: declare 'static' if the function is not intended to be used outside of this translation unit
__attribute__((returns_nonnull)) int* p(int*p) {
                                 ^
                                 static 
1 warning generated.
daniel@laptop:~/cppchecksolutions/cppcheck$ cat 1.cpp

__attribute__((returns_nonnull)) int* p(int*p) {
    if (!p) {}
    return p;
}

So it would be a bad idea to remove the safety checks that checks the return value?

[firewave]

hmm it does not seem that clang/gcc warns properly when pointer could be NULL

There is an upcoming static analyzer check: llvm/llvm-project@4f33e7c.

So it would be a bad idea to remove the safety checks that checks the return value?

The checks we remove should not change much - if we do not want that then we need to add 100+ checks to the code.

[firewave]

We could also detect it ourselves: https://trac.cppcheck.net/ticket/13048.

Enabling the Clang Static Analyzer is looked at in #6835.

[firewave]

No further comments in over a month - merging.