windows.cfg: Add CRITICAL_SECTION handling functions.

[versat]

I have some problems configuring the CRITICAL_SECTION functions:

How should i define CRITICAL_SECTION for cppcheck?
It is a struct and only used internally by Windows and looks like this:

typedef struct _RTL_CRITICAL_SECTION {
    PRTL_CRITICAL_SECTION_DEBUG DebugInfo;
    LONG LockCount;
    LONG RecursionCount;
    HANDLE OwningThread;        // from the thread's ClientId->UniqueThread
    HANDLE LockSemaphore;
    ULONG_PTR SpinCount;        // force size on 64-bit systems when packed
} RTL_CRITICAL_SECTION, *PRTL_CRITICAL_SECTION;
typedef RTL_CRITICAL_SECTION CRITICAL_SECTION;

I guess the checks will not detect as much as they could if this is not known to cppcheck.

Since a CRITICAL_SECTION is some sort of resource i wanted to add a resource description:

  <resource>
    <alloc init="true" arg="1">InitializeCriticalSection</alloc>
    <alloc init="true" arg="1">InitializeCriticalSectionAndSpinCount</alloc>
    <alloc init="true" arg="1">InitializeCriticalSectionEx</alloc>
    <dealloc>DeleteCriticalSection</dealloc>
  </resource>

But for code like this:

    CRITICAL_SECTION CriticalSection;
    // Initialize the critical section one time only.
    if (!InitializeCriticalSectionAndSpinCount(&CriticalSection, 0x00000400) )
        return;

    // Release resources used by the critical section object.
    DeleteCriticalSection(&CriticalSection);

cppcheck complains:
error:autovarInvalidDeallocation:The deallocation of an auto-variable results in undefined behaviour. You should only free memory that has been allocated dynamically.
Is there a way to tell cppcheck that there is no "real" allocation/deallocation but init/deinit here?

[danmar]

what would happen if CRITICAL_SECTION is defined as a pointer something like:

typedef void * CRITICAL_SECTION;

that might be a quick hack that works at the moment, but in the long term maybe we should allow the struct.

[danmar]

I probably implemented this check. But I don't remember how it's supposed to work.

[versat]

Even when defining CRITICAL_SECTION as a void pointer cppcheck still complains when the resource configuration exists.
This is some test code:

// Valid code
void test1ok()
{
    CRITICAL_SECTION CriticalSection;
    if (!InitializeCriticalSectionAndSpinCount(&CriticalSection, 0x00000400) )
    {
        return;
    }
    EnterCriticalSection(&CriticalSection);
    LeaveCriticalSection(&CriticalSection);
    DeleteCriticalSection(&CriticalSection);
}

void test2ok()
{
    CRITICAL_SECTION CriticalSection;
    InitializeCriticalSection(&CriticalSection);
    EnterCriticalSection(&CriticalSection);
    DeleteCriticalSection(&CriticalSection);
}

// Invalid code: EnterCriticalSection uses uninitialized CS
void test1error()
{
    CRITICAL_SECTION CriticalSection;
    // cppcheck-suppress uninitvar
    EnterCriticalSection(&CriticalSection);
}

// Invalid code: Missing DeleteCriticalSection call
void test2error()
{
    CRITICAL_SECTION CriticalSection;
    InitializeCriticalSection(&CriticalSection);
    // cppcheck-suppress resourceLeak
}

I run it with
cppcheck --check-library --enable=information --enable=style -v --error-exitcode=1 --inline-suppr --template="{file}:{line}:{severity}:{id}:{message}" --library=windows "windows_lib_criticalsection.cpp"
And get:

Checking windows_lib_criticalsection.cpp ...
Defines:
Includes:
Platform:win64
windows_lib_criticalsection.cpp:14:error:autovarInvalidDeallocation:The deallocation of an auto-variable results in undefined behaviour. You should only free memory that has been allocated dynamically.
windows_lib_criticalsection.cpp:25:error:autovarInvalidDeallocation:The deallocation of an auto-variable results in undefined behaviour. You should only free memory that has been allocated dynamically.
windows_lib_criticalsection.cpp:44:information:unmatchedSuppression:Unmatched suppression: resourceLeak

[versat]

AppVeyor Build failed because github was temporarily not reachable during the build.

[versat]

Since there seems to be no further improvements possible at the moment i suggest to merge this PR.
Thus cppcheck knows at least a little bit about these functions, like noreturn=false, which could help getting better results i guess.
Shall i create a ticket to not forget adding the resource config once this is implemented somehow?

[danmar]

ok I merged it. Feel free to create a ticket.