-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Random support #20
Merged
Merged
Random support #20
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added LocateHandle to EFI_BOOT_SERVICES in order to find a rng handle. Fixed bug with OpenProtocol in EFI_BOOT_SERVICES to work for interfaces other than EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL. Added pseudo null to EFI_GUID since LocateHandle does not require a guid under certain conditions and expects a EFI_GUID* to null. This is easy enough to do but since the wrapper function expects an EFI_GUID it becomes impossible to give null directly. Added random test code to EfiSharp. Until System.Random is added, this requires public access to ImageHandle from UefiApplication.
Adding Equals(object other) requires RhUnbox2 which throws on failing and so is not supported. Same happens with == and != since those require Equals(object other) to be implemented. Settled for Equals(EFI_GUID other) for now. This removes the need for IsNull() which was added in the last commit.
That is with the exception of the version of NextBytes that uses a span since that is not supported current. The only function that might be a bit weird is NextInt64(long, long) since the range can not be stored in an signed format and even with the work around, the result could overflow long. Can it overflow double?
Random.LegacyImpl.cs could probably be added if BitOperations.cs and BitConverter.cs were added. Not totally sure about the Xoshiro implementations. Edit: The calls to BitConverter involve span which is not currently supported and while it appears possible, it is out of the scope of this pull due to the amount of code from System.Private.CoreLib required. |
Ideally, using ToInt64 it would be possible to add everything required in this file for Random.LegacyImpl.cs.
Automatic seed overload of legacy impl is currently disabled since it was causing the program to hang. BitOperations has the intrinsic sections commented out for convenience. These should be enabled at some point, even if just for the PlatformNotSupported versions.
Adding Random.Xoshiro128StarStarImpl.cs would be easy but since the rest of the corelib only supports 64bit it would be pointless. It is currently not possible to match dotnet behaviour and decide which prng system to use since that is done by checking if the Random instance is derived or not and that requires more Type and EETypePtr support.
There appears to be some hanging issues when using static variables. This might be the root cause for some of the array issues that have caused crashes.
Removed the pseudo null from EFI_GUID. Will revisit if nullable<T> is added. Cleaned up a few more things including todos, memory freeing and unneeded unsafe sections.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Not yet finished since only Random.NextBytes is supported