I got an interested answer in a forum question which I participated in. The question was not about the subject of this article but instead an answer contained code that was interesting enough to write this article. The code was Delphi but it was clearly inspired by plain C code (even using goto). However, here, I want to talk about how errors are returned in most of the WinAPI functions and why the given code in the answer is not the correct way to do so.
The code was not posted in this exact way. I just extracted the necessary parts for your comprehension (The original source was even more a duplicate of real plain C code.):
function IsUserMemberOf(const Group : PSID) : BOOL;
var
hToken : HANDLE;
begin
result := false;
//retrieve Token here and store in hToken ... (short version)
if not OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY or TOKEN_DUPLICATE, hToken) then
goto Exit;
//additional stuff for Vista and house keeping for Windows in general and a lot of gotos
if not CheckTokenMembership(hToken, Group, @result) then
begin
goto Exit_1;
end;
:Exit_1
CloseHandle(hToken);
goto Exit;
:Exit
end;
What is the problem here with the return value of the function? First of all, every WinAPI function is checked against its return value and the function exits if the function call fails. This is good, because I still see a lot of codes where no checks are made. But we have another problem here. There are two possible results (sets of possible return values) we actually want to tell the caller of this function:
The codomain of your function is a binary set which means the range of the return value is FALSE (0) to TRUE (1). However, the function above maps two sets with different meanings into one set: the codomain or function result. If one of the WinAPI calls fails, the function immediately exits and returns FALSE. So the element FALSE is obviously used for two computation results. On that tells the caller that the user is NOT a member of the given group and the other one tells her that the function failed for whatever reason. So we have a mapping of two values to one value. We lost information! Without an additional (meta) information we are unable to distinguish them again. Always think of it when you want to specifiy/write a function. But of course we have this information somewhere else. The WinAPI itself has it. GetLastError provides the error value which (hopefully) helps us to solve the problem. So everything is fine now? I wish! Check this out:
function GetMyUserName : String;
var
nSize : Cardinal;
p : PChar;
begin
nSize := 0;
if not GetUserName(nil, nSize) and (GetLastError() = ERROR_INSUFFICIENT_BUFFER) then
begin
GetMem(p, nSize * sizeof(WCHAR));
try
if not GetUserName(p, nSize) then
RaiseLastOSError;
result := p;
finally
FreeMem(p);
end;
end
else
RaiseLastOSError;
end;
begin
...
xy := GetMyUserName();
bIsMember := IsUserMemberOf(aGroup);
if not bIsMember and (GetLastError() _unequal_ 0) then
ERROR //...handling;
end;
The function IsUserMemberOf is called in succession of GetMyUserName which asks the WinAPI function GetUserName to return the size of the username string. The GetLastError code is set to ERROR_INSUFFICIENT_BUFFER (122dec) which tells us to use create a memory block of valid size.
Now GetMyUserName returns correctly our username and let’s say that IsUserMemberOf does not fail either. But it wants to tell us that the user (or better process) is not the member of our given group. So the return value is FALSE and it is a fact that most of the WinAPI functions won’t change the LastError value if they succeed. So GetLastError will still be ERROR_INSUFFICIENT_BUFFER (122dec) when the variable bIsMember is tested for validity and the result is an ERROR handling which, in fact, should not be done here.
As you see, although we got additional information from GetLastError, the implementation of our function does not allow a caller to find out whether the return value is the value of a computation or an error status.
If you intend to write such a function. I urge you not to use plain C techniques. Why? Let us check how WinAPI functions could look like in Delphi with plain C techniques:
function GetUserNameW(lpBuffer: LPWSTR; nSize: PDWORD): BOOL;
var
pUserName : PWideChar;
res: NTSTATUS;
bRes : Boolean;
begin
bRes := FALSE;
if (nSize = nil) then
begin
SetLastError(ERROR_INVALID_PARAMETER);
goto Exit;
end;
if (lpBuffer = NIL) or (nSize^ _smallerthan_ InternalGetMinUserNameLength()) then
begin
nSize^ := InternalGetMinUserNameLength();
SetLastError(ERROR_INSUFFICIENT_BUFFER);
goto Exit;
end;
//This is not quite necessary but adds more complexion to the example
pUserName := HLOCAL(LocalAlloc(LPTR, InternalGetMinUserNameLength() * sizeof(WIDECHAR));
//TODO: also check pUserName for nil, if nil goto Exit
res := NtGetUserName(pUserName, InternalGetMinUserNameLength() * sizeof(WIDECHAR));
if NT_FAILED(res) then
begin
SetLastError(NtStatusToDosError(res));
goto Exit_1;
end;
if not StrCopyMemory(lpBuffer, pUserName) then //fake call but already uses SetLastError
goto Exit_1;
bRes := TRUE;
:Exit_1
FreeMem(pUserName);
goto Exit;
:Exit
result := bRes; //in C actually: return bRes;
end;
Looks complicated, doesn’t it? I didn’t intend to write the real implemention of GetUserName (I don’t know it by heart) but instead just show a fictive implementation. So WinAPI uses solely the function result as an error indication. FALSE means an error occured and the out parameter values and GetLastError must be checked according to documentation. TRUE means, everything is fine.
Do we need this in Delphi? Not really. If you intend to write a library, a DLL file, that needs to be called by other languages, well, than you could use SetLastError as an error reporting facility. But then you still have to return all your computations in an out/var parameter (by reference). Of course, you can also use the type HRESULT as error indicator in the return value.
function GetUserNameW(lpBuffer: LPWSTR; nSize: PDWORD): HRESULT;
If you want a plain Delphi function, you can make life easier by using exceptions.
procedure GetUserNameW(Name: PWideChar; var nSize: DWORD);
In addition, this makes it possible to return a computation in the function return value.
function GetUserNameW(Name: PWideChar; var nSize: DWORD) : PWideChar;
begin
if (nSize = nil) then
begin
SetLastError(ERROR_INVALID_PARAMETER);
RaiseLastOsError;
end;
... //to make it short one example here is sufficient
end;
As you can see, RaiseLastOsError is called which raises an EOsError. It contains the GetLastError return code in its property ErrorCode automatically. We can even create our own exception class (JWSCL does with EJwsclWinCallFailedException) and get rid of SetLastError at all! And, of course you can use the native string type feature of Delphi which makes the function really easy to use.
function GetUserName() : String;
The function returns a string. It does not return an empty string in a case of failure as other implementations we can find on the internet. Otherwise, we would have the same problem as in the beginning of this article: One return value that can be a valid computation result or an error status. What’s your guess in this situation?
function IsUserMemberOf(const Group : PSID) : Boolean;
var
hToken : HANDLE;
begin
result := false;
//retrieve Token here and store in hToken ... (short version)
if not OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY or TOKEN_DUPLICATE, hToken) then
RaiseLastOsError;
//additional stuff for Vista and house keeping for Windows in general and a lot of RaiseLastOsError;
if not CheckTokenMembership(hToken, Group, @result) then
begin
RaiseLastOsError;
end;
end;
A good implementation comes from JEDI Component Library (JCL) in file JclSecurity.pas Forget about all the other implementations!! Use this one:
function IsGroupMember(RelativeGroupID: DWORD): Boolean;
var
psidAdmin: Pointer;
Token: THandle;
Count: DWORD;
TokenInfo: PTokenGroups;
HaveToken: Boolean;
I: Integer;
const
SE_GROUP_USE_FOR_DENY_ONLY = $00000010;
begin
Result := not IsWinNT;
if Result then // Win9x and ME don't have user groups
Exit;
psidAdmin := nil;
TokenInfo := nil;
HaveToken := False;
try
Token := 0;
HaveToken := OpenThreadToken(GetCurrentThread, TOKEN_QUERY, True, Token);
if (not HaveToken) and (GetLastError = ERROR_NO_TOKEN) then
HaveToken := OpenProcessToken(GetCurrentProcess, TOKEN_QUERY, Token);
if HaveToken then
begin
{$IFDEF FPC}
Win32Check(AllocateAndInitializeSid(SECURITY_NT_AUTHORITY, 2,
SECURITY_BUILTIN_DOMAIN_RID, RelativeGroupID, 0, 0, 0, 0, 0, 0,
psidAdmin));
if GetTokenInformation(Token, TokenGroups, nil, 0, @Count) or
(GetLastError <> ERROR_INSUFFICIENT_BUFFER) then
RaiseLastOSError;
TokenInfo := PTokenGroups(AllocMem(Count));
Win32Check(GetTokenInformation(Token, TokenGroups, TokenInfo, Count, @Count));
{$ELSE FPC}
Win32Check(AllocateAndInitializeSid(SECURITY_NT_AUTHORITY, 2,
SECURITY_BUILTIN_DOMAIN_RID, RelativeGroupID, 0, 0, 0, 0, 0, 0,
psidAdmin));
if GetTokenInformation(Token, TokenGroups, nil, 0, Count) or
(GetLastError <> ERROR_INSUFFICIENT_BUFFER) then
RaiseLastOSError;
TokenInfo := PTokenGroups(AllocMem(Count));
Win32Check(GetTokenInformation(Token, TokenGroups, TokenInfo, Count, Count));
{$ENDIF FPC}
for I := 0 to TokenInfo^.GroupCount - 1 do
begin
{$RANGECHECKS OFF} // Groups is an array [0..0] of TSIDAndAttributes, ignore ERangeError
Result := EqualSid(psidAdmin, TokenInfo^.Groups[I].Sid);
if Result then
begin
//consider denied ACE with Administrator SID
Result := TokenInfo^.Groups[I].Attributes and SE_GROUP_USE_FOR_DENY_ONLY
<> SE_GROUP_USE_FOR_DENY_ONLY;
Break;
end;
{$IFDEF RANGECHECKS_ON}
{$RANGECHECKS ON}
{$ENDIF RANGECHECKS_ON}
end;
end;
finally
if TokenInfo <> nil then
FreeMem(TokenInfo);
if HaveToken then
CloseHandle(Token);
if psidAdmin <> nil then
FreeSid(psidAdmin);
end;
end;
function IsAdministrator: Boolean;
begin
Result := IsGroupMember(DOMAIN_ALIAS_RID_ADMINS);
end;
The same can be done using JWSCL.
uses
JwaWindows,
JwsclToken,
JwsclKnownSid,
JwsclUtils;
var
Token : TJwSecurityToken;
begin
JwInitWellKnownSIDs;
Token := TJwSecurityToken.CreateTokenEffective(TOKEN_READ or TOKEN_QUERY or TOKEN_DUPLICATE);
try
Token.ConvertToImpersonatedToken(DEFAULT_IMPERSONATION_LEVEL, MAXIMUM_ALLOWED);
if Token.CheckTokenMembership(JwAdministratorsSID) then
//active member of Administrators
finally
Token.Free;
end;
end;
2 Responses
Thomas Mueller
29|Oct|2010 1One thing to keep in mind when using RaiseLastOsError is that you don’t get the context of the error. e.g. you only know that copying a file failed but not which file you tried to copy where. This is very annoying, so this usually leads to some code that after the error occurred tries to generate an error message using GetLastError and SysErrorMessage and providing the context. Add to that using resource strings or a tool like dxgettext for translation purposes and you most likely end up getting an error message that has nothing to do with the actual error because an intermediate windows API call changed the GetLastError value before the program actually retrieved it.
So, always the first thing to do after an API call failed, should be storing the result of GetLastError in a local variable. Only then can you be sure that you get the correct error message for the error you want to report.
Chris
29|Oct|2010 2“So, always the first thing to do after an API call failed, should be storing the result of GetLastError in a local variable.”
No need, since the exception raised by RaiseLastOSError carries the error code.
try
//blah
except
on E: EOSError do
//use E.ErrorCode…
end;
Leave a reply
You must be logged in to post a comment.