Skip to content
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.

Sign up for GitHub

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

Change the function to retrieve used buffer instead of available buffer #114

Merged
merged 2 commits into from
Oct 11, 2023
Merged

Change the function to retrieve used buffer instead of available buffer #114

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ea042ce
Change the function to retrieve used buffer instead of available buffer
Oct 11, 2023
dd1d8d8
Use 'available' to mean bytes available to read.
qwandor Oct 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/device/socket/connectionmanager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,10 +226,13 @@ impl<H: Hal, T: Transport> VsockConnectionManager<H, T> {
Ok(bytes_read)
}

/// Returns the number of bytes currently available in the recv buffer.
pub fn recv_buffer_available_bytes(&mut self, peer: VsockAddr, src_port: u32) -> Result<usize> {
/// Returns the number of used bytes in the receive buffer.
///
/// When the used bytes is 0, it indicates that the receive buffer is empty and does not
/// contain any data.
pub fn recv_buffer_used_bytes(&mut self, peer: VsockAddr, src_port: u32) -> Result<usize> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The behaviour is now good, but I think we should call it available, because from the point of view of the caller it is the number of bytes which are available to read. I'll upload another commit to fix this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I feel it confusing to call it available. Especially the same word is used in the same function to indicate the free space in buffer .

Copy link
Collaborator

Choose a reason for hiding this comment

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

Fair point about the method on RingBuffer, we could rename that to free or something if you like. But I really think that 'available' is the best word for the number of bytes which are ready to be read by the caller. The caller shouldn't usually need to care about the details of how the buffering works, all it needs to know is how much data is available to read.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The caller shouldn't usually need to care about the details of how the buffering works, all it needs to know is how much data is available to read.

Agreed. I just don't like using "available" twice in the same code to convey two contradictory meanings for the buffer. It's easier to understand if we adopt consistent naming, or include available_to_read in the name.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay, uploaded #115, PTAL.

let (_, connection) = get_connection(&mut self.connections, peer, src_port)?;
Ok(connection.buffer.available())
Ok(PER_CONNECTION_BUFFER_CAPACITY - connection.buffer.available())
}

/// Sends a credit update to the given peer.
Expand Down
Loading