Skip to content

Improve process_irp_create#12830

Merged
ibeckermayer merged 126 commits intowindows-desktop-directory-sharingfrom
isaiah/irp-mj-create-refactor
Jun 28, 2022
Merged

Improve process_irp_create#12830
ibeckermayer merged 126 commits intowindows-desktop-directory-sharingfrom
isaiah/irp-mj-create-refactor

Conversation

@ibeckermayer
Copy link
Copy Markdown
Contributor

Taking an opportunity I noticed to improve code quality of process_irp_create now that I've refactored err_code to be a TdpErrCode enum.

Isaiah Becker-Mayer added 30 commits March 31, 2022 15:35
…and cliprdr::Client's have the vchan::Client as a field.
…o trigger it by right-clicking, however it isn't working. One reason is that the vchannel PDU header isn't being added (see rdpdr::encode_message for how that's added to other messages). Noticing that made me notice that there is another cliprdr function for breaking outgoing messages into chunks that should be refactored into vchan to do that work + add the necessary vchan headers. This is a checkpoint commit while I go attend to that.
…lient parses the DeviceCreateRequest that's immediately sent back
…ugh in most cases, no need to neurotically add every bit of the documentation text to the code itself
if res.err_code == TdpErrCode::Nil {
return cli.tdp_sd_overwrite(rdp_req, res.fso);
} else {
// The match statement on res.err_code above ensures that this means res.err_code == TdpErrCode::DNE
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, these seems fragile. As soon as you change the branches of the match this breaks. Should we just be more explicit and compare to DNE?

Copy link
Copy Markdown
Contributor Author

@ibeckermayer ibeckermayer Jun 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I believe the reason I opted for this initially is that I didn't want the compiler to force me to add this line:

Err(try_error("Programmer error, this line should never be reached"))

which is needed if those statements are else if's rather than else's.

I've played around with the logic (like wrapping everything in a match res.err_code), and unfortunately haven't come up with a configuration that's logically sound and where the compiler is smart enough to understand it, except for this.

Even though its a bit gross, I think we should go with what I have (or similar) due to the fragility you pointed out.

@ibeckermayer ibeckermayer changed the base branch from isaiah/add-FILE-SUPERSEDE to windows-desktop-directory-sharing June 28, 2022 20:07
@ibeckermayer ibeckermayer merged commit de934c8 into windows-desktop-directory-sharing Jun 28, 2022
ibeckermayer pushed a commit that referenced this pull request Jul 6, 2022
ibeckermayer pushed a commit that referenced this pull request Jul 13, 2022
ibeckermayer pushed a commit that referenced this pull request Aug 23, 2022
* `IRP_MJ_CREATE` (#12665)

* `IRP_MJ_QUERY_INFORMATION` (#12717)

* `IRP_MJ_CLOSE` (#12729)

* Refactor rdpdr client (#12750)

* Adding logic for `FILE_SUPERSEDE` (#12829)

* Improve `process_irp_create` (#12830)

* adds return statements that got lost in a merge

* `IRP_MJ_DIRECTORY_CONTROL` (#12870)

* `FileFullDirectoryInformation` (#12908)

* Improve `ClientDriveQueryDirectoryResponse.encode()` (#12912)

* `IRP_MJ_QUERY_VOLUME_INFORMATION` (#13071)

* Fix Shared Directory Request handling when feature is disabled (#13439)

* IRP_MJ_READ, IRP_MJ_WRITE, and IRP_MJ_SET_INFORMATION (#13995)

* Adds constants for sizing calculations (#14051)

Co-authored-by: Łukasz Kozłowski <lukasz.kozlowski@goteleport.com>
Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
ibeckermayer pushed a commit that referenced this pull request Aug 23, 2022
* `IRP_MJ_CREATE` (#12665)

* `IRP_MJ_QUERY_INFORMATION` (#12717)

* `IRP_MJ_CLOSE` (#12729)

* Refactor rdpdr client (#12750)

* Adding logic for `FILE_SUPERSEDE` (#12829)

* Improve `process_irp_create` (#12830)

* adds return statements that got lost in a merge

* `IRP_MJ_DIRECTORY_CONTROL` (#12870)

* `FileFullDirectoryInformation` (#12908)

* Improve `ClientDriveQueryDirectoryResponse.encode()` (#12912)

* `IRP_MJ_QUERY_VOLUME_INFORMATION` (#13071)

* Fix Shared Directory Request handling when feature is disabled (#13439)

* IRP_MJ_READ, IRP_MJ_WRITE, and IRP_MJ_SET_INFORMATION (#13995)

* Adds constants for sizing calculations (#14051)

Co-authored-by: Łukasz Kozłowski <lukasz.kozlowski@goteleport.com>
Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
ibeckermayer pushed a commit that referenced this pull request Sep 7, 2022
…ring) (#15770)

* Windows Desktop Directory Sharing (#13630)

* `IRP_MJ_CREATE` (#12665)

* `IRP_MJ_QUERY_INFORMATION` (#12717)

* `IRP_MJ_CLOSE` (#12729)

* Refactor rdpdr client (#12750)

* Adding logic for `FILE_SUPERSEDE` (#12829)

* Improve `process_irp_create` (#12830)

* adds return statements that got lost in a merge

* `IRP_MJ_DIRECTORY_CONTROL` (#12870)

* `FileFullDirectoryInformation` (#12908)

* Improve `ClientDriveQueryDirectoryResponse.encode()` (#12912)

* `IRP_MJ_QUERY_VOLUME_INFORMATION` (#13071)

* Fix Shared Directory Request handling when feature is disabled (#13439)

* IRP_MJ_READ, IRP_MJ_WRITE, and IRP_MJ_SET_INFORMATION (#13995)

* Adds constants for sizing calculations (#14051)

Co-authored-by: Łukasz Kozłowski <lukasz.kozlowski@goteleport.com>
Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

* `UnixPath` and `WindowsPath` (#14267)

* `SharedDirectoryMoveRequest` and `SharedDirectoryMoveResponse` (#14959)

* `SharedDirectoryCreateResponse` update (#15289)

* Fix `process_irp_set_information` (#15364)

* Sanitize Rust Debug Logs (#15743)

* updates rdp-rs ref to include licensing changes

* Updates rdp-rs ref and fixes Cargo

Co-authored-by: Łukasz Kozłowski <lukasz.kozlowski@goteleport.com>
Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
@rosstimothy rosstimothy deleted the isaiah/irp-mj-create-refactor branch December 18, 2025 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants