How should I define and initialize a non-global namespace member variable?

Question

In my Memory.hpp file, I have a namespace boolean named isMHinit:

namespace Memory
{
    static bool isMHInit = false;
    DWORD HookFunction(LPVOID pTarget, LPVOID pDetour, LPVOID pTrampoline, BOOL isWriteCopy);
    static UINT64 CheckCodeAccess(UINT64 startaddress);
    DWORD OnAccessHookFunction(PVOID pTarget, LPVOID pDetour, LPVOID pTrampoline, BOOL isWriteCopy);
}; // namespace Memory

And inside Memory.cpp, I use and modify it:

// this function is a member of Memory, I removed most functionality for brevity.
DWORD HookFunction(LPVOID pTarget, LPVOID pDetour, LPVOID pTrampoline, BOOL isWriteCopy)
{
    if (!isMHInit) {
        if (MH_Initialize() != MH_OK) {
            std::cout << "Error: MHInit failed." << "\n";
            return 1;
        }
        isMHInit = true;
    }

When going through clang-tidy, I get this warning, even though I clearly modify it inside the function:

Variable 'isMHInit' is non-const and globally accessible, consider making it const

Is this an issue with clang-tidy, or should I modify something in my code?

This is a potential issue in your code. Make the variable private and add a public static getter function. — 3CEZVQ
– 3CEZVQ, Commented Aug 8, 2024 at 22:49
A non-constant global variable can be a <expletive deleted> to debug. Because anything can change the value at any time for any reason with no traceability, everything in your program could have side effects. Great fun. Clang tidy wants you to save yourself from this hell. — user4581301
– user4581301, Commented Aug 8, 2024 at 23:07

Eugene · Accepted Answer · 2024-08-08 23:13:52Z

2

You should never declare a variable static in a header (when it is not a class member). When a header contains

static bool isMHInit = false;

this means a separate variable isMHInit in each *.cpp file where the header is included. If you want to have a globally accessible variable shared between all source files, you can declare it as

inline bool isMHInit = false;

However, using non-const globally accessible variables is poor design, and Clang-tidy warns you about it.

If you actually use the variable in a single source file only, then keep your declaration as is but move it to the *.cpp file. This makes the design a little better.

answered Aug 8, 2024 at 23:13

Eugene

7,7762 gold badges26 silver badges37 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Surasia Over a year ago

Thank you! Would defining a "Memory" class and then having isMHInit be a private variable be a better approach?

Eugene Over a year ago

@Surasia It probably would be, but your next problem would be to avoid a globally accessible Memory variable. Alternatively, you may make the variable static inside your function.

Collectives™ on Stack Overflow

How should I define and initialize a non-global namespace member variable?

1 Answer 1

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related