Skip to content

Conversation

@nohajc
Copy link

@nohajc nohajc commented Apr 3, 2025

Description

Fix the makedev function signature according to actual implementation:
https://github.com/apple-oss-distributions/xnu/blob/8d741a5de7ff4191bf97d57b9f54c2f6d4a15585/bsd/sys/types.h#L154
https://developer.apple.com/documentation/kernel/1547212-makedev

Issue: #4360

Checklist

  • Relevant tests in libc-test/semver have been updated
  • No placeholder or unstable values like *LAST or *MAX are
    included (see #3131)
  • Tested locally (cd libc-test && cargo test --target mytarget);
    especially relevant for platforms that may not be checked in CI

@rustbot
Copy link
Collaborator

rustbot commented Apr 3, 2025

r? @tgross35

rustbot has assigned @tgross35.
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

@tgross35
Copy link
Contributor

tgross35 commented Apr 3, 2025

This lgtm, but any reason not to also fix major and minor?

@nohajc
Copy link
Author

nohajc commented Apr 3, 2025

@tgross35 Oh, right. They're not even using dev_t there. Let me fix it. It's so bizarre...

@nohajc nohajc force-pushed the fix-apple-makedev branch from 8bf8de8 to 4159fb5 Compare April 3, 2025 22:10
@nohajc
Copy link
Author

nohajc commented Apr 3, 2025

Well, I just noticed the functions are only compiled for C++. C still uses those macros which are more loosely defined. Not sure that's reason enough to deviate from the type signatures even if they're questionable.

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.

Going through old PRs, sorry I missed this one. I think this change is worth making even if they use untyped macros, given this makes Apple match all the other platforms. We still can't backport unfortunately due to breakage, but this would be good to have for 1.0.

Would you be able to rebase?

@nohajc nohajc force-pushed the fix-apple-makedev branch from 4159fb5 to e14dee5 Compare October 30, 2025 10:45
@rustbot rustbot added the A-CI Area: CI-related items label Oct 30, 2025
@rustbot

This comment has been minimized.

@nohajc nohajc force-pushed the fix-apple-makedev branch from e14dee5 to f480703 Compare October 30, 2025 10:48
@rustbot

This comment has been minimized.

@nohajc
Copy link
Author

nohajc commented Oct 30, 2025

Would you be able to rebase?

Done. :)

@tgross35 tgross35 added the stable-declined This change is breaking, difficult to backport, low priority, or otherwise not relevant for 0.2 label Nov 2, 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.

LGTM, thank you! I think you may need to rebase so CI actually passes

@nohajc nohajc force-pushed the fix-apple-makedev branch from d1cbeb6 to b1142b4 Compare November 2, 2025 09:42
@rustbot
Copy link
Collaborator

rustbot commented Nov 2, 2025

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-CI Area: CI-related items O-macos O-unix S-waiting-on-author stable-declined This change is breaking, difficult to backport, low priority, or otherwise not relevant for 0.2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants