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

Implement major/minor macros, correct mkdev #508

Merged
merged 1 commit into from
Feb 17, 2017

Conversation

aidanhs
Copy link
Contributor

@aidanhs aidanhs commented Feb 13, 2017

It appears that the previous mkdev was based on the kernel headers (https://github.com/torvalds/linux/blob/v4.7/include/linux/kdev_t.h#L6) which (I guess) is the internal kernel dev_t. Scrolling down the file you can see some bitshifting operations to do conversions.

The new implementation(s) are based on musl and glibc, which are in agreement about how dev_t should be handled.

(as it happens I suspect we could omit the shift by 32 since I don't see that in the kernel headers, but doesn't hurt to take the conservative route and mimic the libcs)

@posborne
Copy link
Member

There appear to be some issues with your current implementation; in general the changes seem reasonable.

@aidanhs
Copy link
Contributor Author

aidanhs commented Feb 17, 2017

Sorry, thought it was spurious for some reason. Fixed it now.

@posborne
Copy link
Member

@homu r+

@homu
Copy link
Contributor

homu commented Feb 17, 2017

📌 Commit dbd3b16 has been approved by posborne

@homu
Copy link
Contributor

homu commented Feb 17, 2017

⚡ Test exempted - status

@homu homu merged commit dbd3b16 into nix-rust:master Feb 17, 2017
homu added a commit that referenced this pull request Feb 17, 2017
Implement major/minor macros, correct mkdev

It appears that the previous `mkdev` was based on the kernel headers (https://github.com/torvalds/linux/blob/v4.7/include/linux/kdev_t.h#L6) which (I guess) is the internal kernel dev_t. Scrolling down the file you can see some bitshifting operations to do conversions.

The new implementation(s) are based on [musl](http://git.musl-libc.org/cgit/musl/tree/include/sys/sysmacros.h?id=dbbb3734d8c0176feabd6c46e2e85bbc3b8a60af) and [glibc](https://github.molgen.mpg.de/git-mirror/glibc/blob/20003c49884422da7ffbc459cdeee768a6fee07b/sysdeps/unix/sysv/linux/sys/sysmacros.h#L38), which are in agreement about how dev_t should be handled.

(as it happens I suspect we could omit the shift by 32 since I don't see that in the kernel headers, but doesn't hurt to take the conservative route and mimic the libcs)
@homu homu mentioned this pull request Feb 17, 2017
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.

None yet

3 participants