Conversation
There was a problem hiding this comment.
Pull request overview
This PR attempts to refactor the code by renaming the input_clean_change method to perper_change for improved clarity. However, the new function name contains a critical spelling error. Additionally, the refactoring includes moving some calls to this renamed function to different locations in the code flow and removing redundant context flag setup code that is now handled by the renamed function.
Changes:
- Renamed
input_clean_changetoperper_changein theContextDataimplementation - Updated all call sites to use the new function name
- Added a call to
perper_changeinsetup_max_packetmethod and introduced a new line to set endpoint 0 context flag - Removed the call from
evaluatemethod as it's now called beforeevaluatein the calling functions - Simplified
update_hub_innerby removing manual context flag setup that is now handled byperper_change
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| usb-host/src/backend/kmod/xhci/context.rs | Renames the input_clean_change method to perper_change |
| usb-host/src/backend/kmod/xhci/device.rs | Updates all call sites to use perper_change and refactors context setup logic by removing redundant code and adding the renamed function call before operations that modify input context |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| let dci = Dci::CTRL; | ||
| self.ctx.with_input(|input| { | ||
| let _ = input.control_mut().add_context_flag(1); // Endpoint 0 Context |
There was a problem hiding this comment.
The method call appears to be incorrect. Based on the usage pattern elsewhere in the codebase (lines 208, 209, 415 in this file and line 111 in context.rs), this should be "set_add_context_flag" instead of "add_context_flag". The current code will fail to compile as the method "add_context_flag" does not exist. Additionally, the returned value is being discarded with "let _", but set_add_context_flag likely doesn't return a value that needs to be handled.
| let _ = input.control_mut().add_context_flag(1); // Endpoint 0 Context | |
| input.control_mut().set_add_context_flag(1); // Endpoint 0 Context |
| } | ||
|
|
||
| async fn setup_max_packet(&mut self, desc: DeviceDescriptorBase) -> Result { | ||
| self.ctx.perper_change(); |
There was a problem hiding this comment.
The function name "perper_change" appears to be a typo. This should likely be "prepare_change" or "prepare_input_change" to match the function's purpose of preparing the input context for changes.
| self.ctx.perper_change(); | |
| self.ctx.prepare_change(); |
| } | ||
|
|
||
| async fn _set_configuration(&mut self, configuration_value: u8) -> Result { | ||
| self.ctx.perper_change(); |
There was a problem hiding this comment.
The function name "perper_change" appears to be a typo. This should likely be "prepare_change" or "prepare_input_change" to match the function's purpose of preparing the input context for changes.
| self.ctx.perper_change(); | |
| self.ctx.prepare_change(); |
| } | ||
|
|
||
| async fn _claim_interface(&mut self, interface: u8, alternate: u8) -> Result { | ||
| self.ctx.perper_change(); |
There was a problem hiding this comment.
The function name "perper_change" appears to be a typo. This should likely be "prepare_change" or "prepare_input_change" to match the function's purpose of preparing the input context for changes.
| async fn setup_all_endpoints(&mut self, interface: u8, alternate: u8) -> Result { | ||
| let mut max_dci = 1; | ||
| self.ctx.input_clean_change(); | ||
| self.ctx.perper_change(); |
There was a problem hiding this comment.
The function name "perper_change" appears to be a typo. This should likely be "prepare_change" or "prepare_input_change" to match the function's purpose of preparing the input context for changes.
| control_ctx.set_add_context_flag(0); | ||
| }); | ||
|
|
||
| self.ctx.perper_change(); |
There was a problem hiding this comment.
The function name "perper_change" appears to be a typo. This should likely be "prepare_change" or "prepare_input_change" to match the function's purpose of preparing the input context for changes.
| self.ctx.perper_change(); | |
| self.ctx.prepare_change(); |
| } | ||
|
|
||
| pub fn input_clean_change(&mut self) { | ||
| pub fn perper_change(&mut self) { |
There was a problem hiding this comment.
The function name "perper_change" appears to be a typo. Based on the function's behavior (clearing and preparing input context for changes), this should likely be "prepare_change" or "prepare_input_change" for clarity.
1 participant
…larity