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.

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

Conversation

aliciawyy
Copy link
Contributor

The status of the used receive buffer is more helpful for users when configuring credit updates.

Alice Wang and others added 2 commits October 11, 2023 07:35
The status of the used receive buffer is more helpful for users
when configuring credit updates.
///
/// 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.

@qwandor qwandor enabled auto-merge (squash) October 11, 2023 09:36
@qwandor qwandor merged commit 9a6947e into rcore-os:master Oct 11, 2023
5 checks passed
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