Skip to content

Conversation

@afiffon
Copy link
Owner

@afiffon afiffon commented Sep 13, 2025

No description provided.

@afiffon afiffon requested a review from Copilot September 13, 2025 18:10
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements zero-copy I/O operations for the SMB client by introducing an IoVec abstraction that can hold both owned and shared buffer references. This reduces memory allocations and copying during data transfers.

Key changes:

  • Introduces IoVec and IoVecBuf types for zero-copy buffer management
  • Refactors message signing, encryption, and transport layers to work with IoVec
  • Adds progress reporting capabilities to file copy operations

Reviewed Changes

Copilot reviewed 28 out of 29 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
crates/smb/src/util/iovec.rs New module defining IoVec and IoVecBuf for zero-copy buffer management
crates/smb/src/util.rs Module declaration for iovec utilities
crates/smb/src/session/signer.rs Updated to use IoVec instead of &[u8] for message signing
crates/smb/src/session/encryptor_decryptor.rs Modified encryption to work with IoVec
crates/smb/src/resource/pipe.rs Updated to use zero-copy outgoing messages
crates/smb/src/resource/file_util.rs Added progress reporting and visibility improvements to copy operations
crates/smb/src/resource/file.rs Added zero-copy write methods
crates/smb/src/resource.rs Added sendo_recvo method for zero-copy message handling
crates/smb/src/msg_handler.rs Updated message structures to support IoVec and additional data
crates/smb/src/connection/transformer.rs Refactored to use IoVec throughout transformation pipeline
crates/smb/src/connection/transport/traits.rs Updated transport layer to send IoVec data
crates/smb-msg/src/file.rs Modified WriteRequest to support zero-copy operations
crates/smb-cli/src/copy.rs Enhanced with progress bar support for file operations

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

fn calculate_signature(&mut self, header: &mut Header, raw_data: &[u8]) -> crate::Result<u128> {
fn _calculate_signature(&mut self, header: &mut Header, data: &IoVec) -> crate::Result<u128> {
// Write header with signature set to 0.
let signture_backup = header.signature;
Copy link

Copilot AI Sep 13, 2025

Choose a reason for hiding this comment

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

The variable name 'signture_backup' contains a typo. It should be 'signature_backup'.

Copilot uses AI. Check for mistakes.
Comment on lines +194 to +198
/// pass the return value to start_parallel_copy to start the copy.
///
/// this is mostly useful for cases that require an additional interaction with the
/// state, beyond the copy workers themselves - for example, reporting progress.
/// if you don't need that, just use the [`block_copy`] function directly.
Copy link

Copilot AI Sep 13, 2025

Choose a reason for hiding this comment

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

The documentation comment has inconsistent capitalization. Sentences should start with capital letters: 'Pass the return value to start_parallel_copy to start the copy. This is mostly useful for cases that require an additional interaction with the state, beyond the copy workers themselves - for example, reporting progress. If you don't need that, just use the [block_copy] function directly.'

Suggested change
/// pass the return value to start_parallel_copy to start the copy.
///
/// this is mostly useful for cases that require an additional interaction with the
/// state, beyond the copy workers themselves - for example, reporting progress.
/// if you don't need that, just use the [`block_copy`] function directly.
/// Pass the return value to start_parallel_copy to start the copy.
///
/// This is mostly useful for cases that require an additional interaction with the
/// state, beyond the copy workers themselves - for example, reporting progress.
/// If you don't need that, just use the [`block_copy`] function directly.

Copilot uses AI. Check for mistakes.
Comment on lines +172 to +174
/// # Notes
/// - To report progress, use the [`prepare_parallel_copy`] function to get a `CopyState`, and then
/// use that to report progress while the copy is running.
Copy link

Copilot AI Sep 13, 2025

Choose a reason for hiding this comment

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

The documentation comment has inconsistent punctuation. It should end with a period: '- To report progress, use the [prepare_parallel_copy] function to get a CopyState, and then use that to report progress while the copy is running.'

Copilot uses AI. Check for mistakes.
@afiffon afiffon enabled auto-merge September 13, 2025 18:22
@afiffon afiffon merged commit a64f51d into main Sep 13, 2025
4 checks passed
@afiffon afiffon deleted the feature/zero-copy branch October 10, 2025 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants