c++ccode-analysissal

Code analysis doesn't understand _In_opt_ parameter Annotation?


Looks like SAL bug. The code:

    PAAFILEFILTER_PROTECTED_FILE curFile = NULL;

     try
        {
            status = GetProtectedFile(FileIdInfo, instanceContext, &curFile);
            if(!NT_SUCCESS(status))
            {
                TraceError("Can't GetProtectedFile with status: %!STATUS!\n", status);
                leave;
            }
         ...


    finally
    {
        if(NT_SUCCESS(status))
        {
            LogMessage(AAFILEFILTER_FILE_UNPROTECTED, NULL, NULL, NULL, 0, (PUCHAR)FileIdInfo, sizeof(AAFILE_ID_INFORMATION));
        }
        else
        {
            TraceProtectedFile(curFile);
        }
    }

And code analysys give me C6102 - Using variable from failed function call

at line TraceProtectedFile(curFile); but TraceProtectedFile have prototype

_In_opt_ PAAFILEFILTER_PROTECTED_FILE protectedFile

_In_opt_ mean "_In_opt_ is the same as _In_, except that the input parameter is allowed to be NULL and, therefore, the function should check for this." .. don't undestand if CA can't handle such simple things then what it can :(


Solution

  • This looks like a problem with the way your error handling is structured, not the _In_opt_ parameter.

    I wouldn't be surprised if leave, when mixed with standard C++ exception handling, confuses SAL enough that it doesn't recognize that the finally will never be hit. leave isn't part of standard C++ exceptions and is MSVC-specific, intended for structured exception handling.

    The good thing is that that SAL's confusion is a hint that other developers might be similarly surprised by error handling like this. You should probably consider moving the GetProtectedFile call outside of your try/finally, since all of that code assumes that curFile was initialized successfully:

    PAAFILEFILTER_PROTECTED_FILE curFile = NULL;
    
    status = GetProtectedFile(FileIdInfo, instanceContext, &curFile);
    if(!NT_SUCCESS(status))
    {
        TraceError("Can't GetProtectedFile with status: %!STATUS!\n", status);
        return; // Return whatever is appropriate here
    }
    
    // The rest of your code can assume curFile initialized successfully
    
    try
    {  
        ...
    }
    finally
    {
        if(NT_SUCCESS(status))
        {
            LogMessage(AAFILEFILTER_FILE_UNPROTECTED, NULL, NULL, NULL, 0, (PUCHAR)FileIdInfo, sizeof(AAFILE_ID_INFORMATION));
        }
        else
        {
            TraceProtectedFile(curFile);
        }
    }