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

adding tcp_info struct to linux musl/glibc. #3480

Merged
merged 1 commit into from
Sep 3, 2024

Conversation

devnexen
Copy link
Contributor

No description provided.

@rustbot
Copy link
Collaborator

rustbot commented Dec 15, 2023

r? @JohnTitor

(rustbot has picked a reviewer for you, use r? to override)

@JohnTitor
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Dec 15, 2023

📌 Commit 89df119 has been approved by JohnTitor

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Dec 15, 2023

⌛ Testing commit 89df119 with merge 28aeb23...

bors added a commit that referenced this pull request Dec 15, 2023
adding tcp_info struct to linux musl/glibc.
@bors
Copy link
Contributor

bors commented Dec 15, 2023

💔 Test failed - checks-actions

@bors
Copy link
Contributor

bors commented Dec 19, 2023

☔ The latest upstream changes (presumably #3486) made this pull request unmergeable. Please resolve the merge conflicts.

@JohnTitor
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Dec 20, 2023

📌 Commit 97a839d has been approved by JohnTitor

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Dec 20, 2023

⌛ Testing commit 97a839d with merge 3c6c040...

bors added a commit that referenced this pull request Dec 20, 2023
adding tcp_info struct to linux musl/glibc.
@bors
Copy link
Contributor

bors commented Dec 20, 2023

💔 Test failed - checks-actions

@JohnTitor
Copy link
Member

Could you check the CI failure? Seems musl gets confused 🤔

@JohnTitor
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Dec 29, 2023

📌 Commit 4234bd2 has been approved by JohnTitor

It is now in the queue for this repository.

@devnexen
Copy link
Contributor Author

devnexen commented Sep 1, 2024

@rustbot review

pub tcpi_backoff: u8,
pub tcpi_options: u8,
/*
* FIXME: when musl headers are more up to date
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* FIXME: when musl headers are more up to date
* FIXME(musl): when musl headers are more up to date

Just so we can grep these things easy at the musl update

pub tcpi_bytes_retrans: u64,
pub tcpi_dsack_dups: u32,
pub tcpi_reord_seen: u32,
// FIXME: to uncomment once CI musl is updated
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// FIXME: to uncomment once CI musl is updated
// FIXME(musl): to uncomment once CI musl is updated

Same as above

Comment on lines 466 to 469
// bitfields 4
pub tcpi_snd_wscale: u8,
// bitfields 4
pub tcpi_rcv_wscale: u8,
Copy link
Contributor

@tgross35 tgross35 Sep 3, 2024

Choose a reason for hiding this comment

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

The C header has these as

uint8_t	tcpi_snd_wscale : 4, tcpi_rcv_wscale : 4;

doesn't that mean tcpi_snd_wscale and tcpi_rcv_wscale are located within the same u8, and should be a single field in the Rust struct? Feel free to correct me if this is wrong.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was trying to find what we do here in other cases, but it looks like this doesn't come up very often. Probably just make it one field and add a comment so users know:

/// The bitfield containing `tcpi_snd_wscale` and `tcpi_rcv_wscale`, four bits each.
pub tcpi_snd_rcv_wscale: u8

The musl section that is commented out should probably say something similar.

@tgross35
Copy link
Contributor

tgross35 commented Sep 3, 2024

src/unix/linux_like/linux/gnu/mod.rs Outdated Show resolved Hide resolved
src/unix/linux_like/linux/musl/mod.rs Outdated Show resolved Hide resolved
src/unix/linux_like/linux/musl/mod.rs Outdated Show resolved Hide resolved
@tgross35
Copy link
Contributor

tgross35 commented Sep 3, 2024

Thanks!

@rustbot label +stable-nominated

@rustbot rustbot added the stable-nominated This PR should be considered for cherry-pick to libc's stable release branch label Sep 3, 2024
@tgross35 tgross35 enabled auto-merge September 3, 2024 19:29
@tgross35 tgross35 added this pull request to the merge queue Sep 3, 2024
Merged via the queue into rust-lang:main with commit 2899529 Sep 3, 2024
41 checks passed
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Oct 15, 2024
@tgross35 tgross35 mentioned this pull request Oct 15, 2024
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Oct 15, 2024
@tgross35 tgross35 added stable-applied This PR has been cherry-picked to libc's stable release branch and removed stable-nominated This PR should be considered for cherry-pick to libc's stable release branch labels Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-ci stable-applied This PR has been cherry-picked to libc's stable release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants