_Crypt_GenRandom always returns False and sets @error

[wraithdu]

The function sets $iError = @error + 10 without checking @error. Further, I see no documentation or reason for @error + 10 at all.

Func _Crypt_GenRandom($pBuffer, $iSize)
	_Crypt_Startup()
	Local $aRet = DllCall(__Crypt_DllHandle(), "bool", "CryptGenRandom", "handle", __Crypt_Context(), "dword", $iSize, "struct*", $pBuffer)
	Local $iError = @error + 10, $iExtended = @extended
	_Crypt_Shutdown()
	If $iError Or (Not $aRet[0]) Then
		Return SetError($iError, $iExtended, False)
	Else
		Return True
	EndIf
EndFunc   ;==>_Crypt_GenRandom

[BrewManNH]

According to the header in the function, you wrote it, did someone change it after it was committed?

[wraithdu]

Yep. My original version is in ticket #2263.

[BrewManNH]

I found where it was changed in the repository. Not sure why it got changed the way it did, but easy enough to correct.

[BrewManNH]

Fixed by revision [11324] in version: 3.3.13.21

[J-Paul Mesnage]

In fact the + 10 if the $aRet[0] is non zero to insure the setting of a non zeror @error

[jchd18]

In every function in Crypt.au3 error return values are offset by distinct constant values; I guess it's to help locate where the error popped from.

[BrewManNH]

That doesn't explain why it is set that way, or when it was changed, or where it was documented. If you make a change like this, it needs to be documented.

Where 10 is being added to the @error value is definitely a bug. If you wanted to return a non-zero value for the $aRet[0] being 0 then you should be adding 10 to the $iError variable AFTER the If statement, not before it. The way it was rewritten guarantees that it will always return an error condition unless @error is -10.

[BrewManNH]

Fixed by revision [11327] in version: 3.3.13.21

[J-Paul Mesnage]

Thanks ,
You right checking only @error instead of $iError was the smallest correction

[jchd18]

I never pretended it was the right way to handle errors and never said it wasn't a bug! And I never touched this code.