IOS: Clean up the way IPC replies are constructed #9508
Merged
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
leoetlino
commented
Feb 13, 2021
| @@ -57,13 +57,13 @@ void DIDevice::DoState(PointerWrap& p) | |||
| p.Do(m_last_length); | |||
| } | |||
|
|
|||
| IPCCommandResult DIDevice::Open(const OpenRequest& request) | |||
| std::optional<IPCReply> DIDevice::Open(const OpenRequest& request) | |||
There was a problem hiding this comment.
Quick guide to reviewing:
- command handlers return
std::optional<IPCReply>orIPCReply - IPCCommandResult -> IPCReply
- GetNoReply -> std::nullopt
- GetDefaultReply -> IPCReply
Instead of constructing IPCCommandResult with static member functions in the Device class, we can just add the relevant constructors to the reply struct itself. Makes more sense than putting it in Device when the struct is used in the kernel code and doesn't use any Device specific members... This commit also changes the IPC command handlers to return an optional IPCCommandResult rather than an IPCCommandResult. This removes the need for a separate boolean that indicates whether the "result" is actually a reply, and also avoids the need to set dummy result values and ticks. It also makes it really obvious which commands can result in no reply being generated. Finally, this commit renames IPCCommandResult to IPCReply since the struct is now only used for actual replies. This new name is less verbose in my opinion. The diff is quite large since this touches every command handler, but the only functional change is that I fixed EnqueueIPCReply to take a s64 for cycles_in_future to match IPCReply.
leoetlino
force-pushed
the
ipc-reply-cleanup
branch
from
c873d08
to
d0136dd
Compare
BhaaLseN
approved these changes
Feb 13, 2021
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.
Instead of constructing IPCCommandResult with static member functions
in the Device class, we can just add the relevant constructors to the
reply struct itself. Makes more sense than putting it in Device when the
struct is also used in the kernel code and doesn't use any Device
specific members...
This commit also changes the IPC command handlers to return an optional
IPCCommandResult rather than an IPCCommandResult. This removes the need
for a separate boolean that indicates whether the "result" is actually
a reply, and also avoids the need to set dummy result values and ticks.
It also makes it really obvious which commands can result in no reply
being generated.
Finally, this commit renames IPCCommandResult to IPCReply since the
struct is now only used for actual replies. This new name is less
verbose in my opinion.
The diff is quite large since this touches every command handler, but
the only functional change is that I fixed EnqueueIPCReply to
take a s64 for cycles_in_future to match IPCReply.