Skip to content

std: sys: net: uefi: tcp4: Implement write #141532

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

Merged
merged 1 commit into from
Jul 5, 2025

Conversation

Ayush1325
Copy link
Contributor

A blocking implementation of tcp4 write.

@rustbot
Copy link
Collaborator

rustbot commented May 25, 2025

r? @thomcc

rustbot has assigned @thomcc.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels May 25, 2025
@Ayush1325
Copy link
Contributor Author

@rustbot label +O-UEFI

@rustbot rustbot added the O-UEFI UEFI label Jun 15, 2025
Copy link
Contributor

@tgross35 tgross35 left a comment

Choose a reason for hiding this comment

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

A few nits but this looks reasonable to me. @nicholasbishop would you mind double checking since this is UEFI?

let protocol = self.protocol.as_ptr();
let mut token = tcp4::IoToken {
completion_token,
packet: tcp4::IoTokenPacket { tx_data: &mut tx_data as *mut _ as *mut _ },
Copy link
Contributor

Choose a reason for hiding this comment

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

            packet: tcp4::IoTokenPacket { tx_data: ptr::from_mut(&mut tx_data).cast::<whatever>() },

What casts are needed? tx_data should be tcp4::TransmitData, and that looks like what is expected in https://docs.rs/r-efi/latest/r_efi/protocols/tcp4/union.IoTokenPacket.html.

Also, isn't rx_data missing?

Copy link
Contributor Author

@Ayush1325 Ayush1325 Jul 2, 2025

Choose a reason for hiding this comment

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

What casts are needed? tx_data should be tcp4::TransmitData, and that looks like what is expected in https://docs.rs/r-efi/latest/r_efi/protocols/tcp4/union.IoTokenPacket.html.

Well, tcp4::TransmitData is a const generic. tcp4::IoTokenPacket takes ZST (tcp4::TransmitData<0>). So we are casting tcp4::TransmitData<1> to tcp4::TransmitData<0>.

Also, isn't rx_data missing?

It's a union.


let fragment = tcp4::FragmentData {
fragment_length: data_len,
fragment_buffer: buf.as_ptr() as *mut _,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit to make the cast obvious (especially since it includes a mutability change)

            fragment_buffer: buf.as_ptr().cast::<c_void>().cast_mut()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

@tgross35 tgross35 assigned tgross35 and unassigned thomcc Jul 1, 2025
pub fn write_vectored(&self, _: &[IoSlice<'_>]) -> io::Result<usize> {
unsupported()
pub fn write_vectored(&self, buf: &[IoSlice<'_>]) -> io::Result<usize> {
// FIXME: UEFI does support vectored write, so implment that.
Copy link
Contributor

Choose a reason for hiding this comment

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

implment -> implement

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

let evt = unsafe { self.create_evt() }?;
let completion_token =
tcp4::CompletionToken { event: evt.as_ptr(), status: Status::SUCCESS };
let data_len = crate::cmp::min(u32::MAX as u64, buf.len() as u64) as u32;
Copy link
Contributor

Choose a reason for hiding this comment

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

Am I reading correctly that this is truncating the buffer length to a u32? If so, might be clearer as:

let data_len = u32::try_from(buf.len()).unwrap_or(u32::MAX);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

let mut token = tcp4::IoToken {
completion_token,
packet: tcp4::IoTokenPacket {
tx_data: &mut tx_data as *mut tcp4::TransmitData<1> as *mut tcp4::TransmitData,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this casting could be simplified:

tx_data: (&raw mut tx_data).cast::<tcp4::TransmitData<0>>(),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

A blocking implementation of tcp4 write.

Signed-off-by: Ayush Singh <[email protected]>
Copy link
Contributor

@tgross35 tgross35 left a comment

Choose a reason for hiding this comment

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

Thanks!

@tgross35
Copy link
Contributor

tgross35 commented Jul 4, 2025

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jul 4, 2025

📌 Commit f4ef1a7 has been approved by tgross35

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 4, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 4, 2025
std: sys: net: uefi: tcp4: Implement write

A blocking implementation of tcp4 write.
bors added a commit that referenced this pull request Jul 4, 2025
Rollup of 9 pull requests

Successful merges:

 - #141532 (std: sys: net: uefi: tcp4: Implement write)
 - #143085 (Port `#[non_exhaustive]` to the new attribute parsing infrastructure)
 - #143298 (`tests/ui`: A New Order [23/N])
 - #143372 (Remove names_imported_by_glob_use query.)
 - #143386 (Assign dependency bump PRs to me)
 - #143406 (Remove some unnecessary `unsafe` in VecCache)
 - #143408 (mbe: Gracefully handle macro rules that end after `=>`)
 - #143414 (remove special-casing of boxes from match exhaustiveness/usefulness analysis)
 - #143444 (clean up GVN TypeId test)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Jul 4, 2025
Rollup of 8 pull requests

Successful merges:

 - #141532 (std: sys: net: uefi: tcp4: Implement write)
 - #143085 (Port `#[non_exhaustive]` to the new attribute parsing infrastructure)
 - #143372 (Remove names_imported_by_glob_use query.)
 - #143386 (Assign dependency bump PRs to me)
 - #143406 (Remove some unnecessary `unsafe` in VecCache)
 - #143408 (mbe: Gracefully handle macro rules that end after `=>`)
 - #143414 (remove special-casing of boxes from match exhaustiveness/usefulness analysis)
 - #143444 (clean up GVN TypeId test)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit edfaaeb into rust-lang:master Jul 5, 2025
11 checks passed
@rustbot rustbot added this to the 1.90.0 milestone Jul 5, 2025
rust-timer added a commit that referenced this pull request Jul 5, 2025
Rollup merge of #141532 - Ayush1325:uefi-tcp4-send, r=tgross35

std: sys: net: uefi: tcp4: Implement write

A blocking implementation of tcp4 write.
@Ayush1325 Ayush1325 deleted the uefi-tcp4-send branch July 5, 2025 02:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-UEFI UEFI S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants