C & C++

C & C++

Made by DeepSource

Audit required: found a non-const global variable CXX-W2009

Anti-pattern
Minor

Global variables are accessible anywhere in the scope of the program even in the threads outside of the main thread, as long as it shares the main process stack.

A mutable global variable have high dependency and the dependent code on such variables have no way to learn about the changes to the variable. Hence using a non-const global variables is considered quite risky, as they can lead to data races or contention issues.

Consider avoiding the use of global variable by adding a qualifier const. If the variable must be mutable then consider passing the variable as argument to the functions which are designed to mutate the variable.

In the case of atomic or synchronization primitives being used to handle data races, there is still concern about misuse, which can be reduced by making it non-global. That is a private class member that can only be used by its function.

Consider passing references to unique values and then merging them as if you need mutable stack of program memory.

Note that const pointers and pointer to const are not the same. You need a global pointer to be entirely const for this lint to be satisfied.

That is, if you are passing a pointer, then you need to make sure that the pointer is a const pointer to a const. (See here for more information.)

const int *p = &x;
// should instead be
const int *const p = &x;

Bad practice

std::atomic<bool> flag(false);

int global_counter = 0;
void foo() {
    global_counter += 1;
    flag.store(true);
}

int main() {
    global_counter += 1;
    std::thread t(foo);
    flag.store(false); // there won't be a data race,
    // but the output can still be unexpected as we
    // don't know which mutation will occur last
    t.join();
}

Recommended

#include <thread>

int main() {
    int global_counter = 1;
    int global_counter1 = 0;
    std::atomic<bool> flag(false);
    std::thread t1([&](){
        for (int i = 0; i < 10; i++) {
            global_counter1 += 1;
            flag.store(true);
        }
    });
    int global_counter2 = 0;
    std::thread t2([&](){
        for (int i = 0; i < 10; i++) {
            global_counter2 += 1;
            flag.store(false);
        }
    });
    t1.join();
    t2.join();
    // we again don't know which one executed
    // last but we know nothing else but things
    // from this scope can use it making
    // debugging easier
    global_counter += global_counter1 + global_counter2;
}

References