Explain what problems could have this function (if any)

Question

SCENARIO

When P/Invoking, I thinked it could be a great idea to simplify/reduct tons of code by designing a generic function that calls the function, then it checks for the GetLastWin32Error

I'm using this code:

''' <summary>
''' Invokes the specified encapsulated function, trying to provide a higher safety level for error-handling.
''' If the function that was called using platform invoke has the <see cref="DllImportAttribute.SetLastError"/>,
''' then it checks the exit code returned by the function, and, if is not a success code, throws a <see cref="Win32Exception"/>.
''' </summary>
''' ----------------------------------------------------------------------------------------------------
''' <typeparam name="T"></typeparam>
''' 
''' <param name="expr">
''' The encapsulated function.
''' </param>
''' ----------------------------------------------------------------------------------------------------
''' <returns>
''' The type of the return value depends on the function definition.
''' </returns>
''' ----------------------------------------------------------------------------------------------------
''' <exception cref="Win32Exception">
''' Function 'X' thrown an unmanaged Win32 exception with error code 'X'.
''' </exception>
''' ----------------------------------------------------------------------------------------------------
<DebuggerStepThrough>
Private Shared Function SafePInvoke(Of T)(ByVal expr As Expression(Of Func(Of T))) As T

    Dim result As T = expr.Compile.Invoke()

    Dim method As MethodInfo =
        CType(expr.Body, MethodCallExpression).Method

    Dim isSetLastError As Boolean =
        method.GetCustomAttributes(inherit:=False).
               OfType(Of DllImportAttribute)().FirstOrDefault.SetLastError

    If isSetLastError Then

        Dim exitCode As Integer = Marshal.GetLastWin32Error

        If exitCode <> 0 Then
            Throw New Win32Exception([error]:=exitCode,
                                     message:=String.Format("Function '{0}' thrown an unmanaged Win32 exception with error code '{1}'.",
                                                            method.Name, CStr(exitCode)))
        End If

    End If

    Return result

End Function

The things I think to make this efficient should be that the API function should be able to set the last error, and, when the function is able to do it, then I should set the SetLastError attribute to True, of course a SetLastError=True will be ignored if the function by naturally doesns't set a last error, so anyways no matter what kind of funtion I'll pass to this generic SafePinvoke function, it will not give a "false positive", or at least I think not.

Then, an usage example should be this:

• Firstly, we look for an API func that manages the last error, for example FindWindow

• Secondly, we add the definition in our code, setting the SetLastError attribute in the signature.

  <DllImport("user32.dll", SetLastError:=True)>
  Private Shared Function FindWindow(
                          ByVal lpClassName As String,
                          ByVal zero As IntPtr
  ) As IntPtr
  End Function
  

• Finally, we use it.

  Dim lpszParentClass As String = "Notepad"
  
  Dim parenthWnd As IntPtr =
      SafePInvoke(Function() FindWindow(lpszParentClass, IntPtr.Zero))
  
  If parenthWnd = IntPtr.Zero Then
      MessageBox.Show(String.Format("Window found with HWND: {0}", CStr(parenthWnd)))
  
  Else
      MessageBox.Show("Window not found.")
  
  End If
  

At this point we can see that all appear to work as expected, if the window is found it will return a non Zero Intptr, if the window is not found it will return an Intptr.Zero, and, if the function fails because an empty string, it will throw a Win32Exception with error code 123 which reffers to:

ERROR_INVALID_NAME

123 (0x7B)

The filename, directory name, or volume label syntax is incorrect.

All seems ok.

QUESTION

I need to say this to argument the reason of my question, I will not cause anything negative, but just what happened is that some very experienced programmers said that my function is not any safe because I'm wrong with many things about GetLastWin32Error, in this thread:

https://stackoverflow.com/questions/30878232/check-at-run-time-whether-a-p-invoke-function-has-the-dllimportattribute-setlast/30881540?noredirect=1#comment49821007_30881540

My intention is to learn from my errors, but for this, first I should encounter the evidence of that errors, and I didn't found it.

I would like to improve or in case of needed totally delete and re-think the approach of the generic function SafePinvoke above if really it could not work as expected in "X" circunstances, just I would like to see and learn what circunstances could be that, by giving a real code sample that can be tested to demonstrate an error/conflict, then, my question is:

Someone could illustrate with a real code-example of an API function that , when passed through the SafePinvoke function above, it could give a "false positive" error or other kind of conflict?.

Someone could guide me and explain me if the the SafePinvoke function it is really safe, or is not safe, or if it can be improved, and that?, providing a code example that can be tested?.

I will be very gratefull for all kind of information that could helps me to improve this approach, or to learn that the approach really will not work in some circumstances, but please, giving a code example that demonstrate it.

Answer 1

From the documentation of GetLastError:

The Return Value section of the documentation for each function that sets the last-error code notes the conditions under which the function sets the last-error code. Most functions that set the thread's last-error code set it when they fail. However, some functions also set the last-error code when they succeed. If the function is not documented to set the last-error code, the value returned by this function is simply the most recent last-error code to have been set; some functions set the last-error code to 0 on success and others do not.

In other words, it is a mistake to call GetLastError unconditionally. That GetLastError returns a non-zero value does not indicate that the latest API call failed. Moreover, not does GetLastError returning zero indicate success.