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

add major/minor on BSDs/illumos #2999

Merged
merged 1 commit into from
May 6, 2023

Conversation

SteveLauC
Copy link
Contributor

@SteveLauC SteveLauC commented Nov 11, 2022

This PR adds major/minor on BSDs and major/minor/makedev on illumos.

Ref:

  • FreeBSD 11

    #define	major(x)	((int)(((u_int)(x) >> 8)&0xff))	/* major number */
    #define	minor(x)	((int)((x)&0xffff00ff))		/* minor number */
  • FreeBSD 12/13/14

    static __inline int
    __major(dev_t _d)
    {
        return (((_d >> 32) & 0xffffff00) | ((_d >> 8) & 0xff));
    }
    
    static __inline int
    __minor(dev_t _d)
    {
        return (((_d >> 24) & 0xff00) | (_d & 0xffff00ff));
    }
  • DragonFly

    #define	major(x)	((int)(((u_int)(x) >> 8)&0xff)) /* major number */
    #define	minor(x)	((int)((x)&0xffff00ff))         /* minor number */
  • NetBSD

    #define	major(x)	((devmajor_t)(((uint32_t)(x) & 0x000fff00) >>  8))
    #define	minor(x)	((devminor_t)((((uint32_t)(x) & 0xfff00000) >> 12) | \
     			                      (((uint32_t)(x) & 0x000000ff) >>  0)))
  • OpenBSD

    #define	major(x)	(((unsigned)(x) >> 8) & 0xff)
    #define	minor(x)	((unsigned)((x) & 0xff) | (((x) & 0xffff0000) >> 8))
  • illumos:

    1. mkdev.c
    2. mkdev.h

@rust-highfive
Copy link

Some changes occurred in solarish module

cc @jclulow,@pfmooney

Some changes occurred in OpenBSD module

cc @semarie

@rust-highfive
Copy link

r? @JohnTitor

(rust-highfive has picked a reviewer for you, use r? to override)

@pfmooney
Copy link
Contributor

* [illumos](https://github.com/illumos/illumos-gate/blob/8b26092d555bd1deaacf79ea64da374602aefb65/usr/src/boot/sys/sys/types.h#L379-L381)
  ```c
  #define	major(x)	((int)(((u_int)(x) >> 8)&0xff))	/* major number */
  #define	minor(x)	((int)((x)&0xffff00ff))		/* minor number */
  #define	makedev(x,y)	((dev_t)(((x) << 8) | (y)))	/* create dev_t */
  ```

Those are not the major/minor/makdev macros for illumos itself. Although it's not advertised especially well, usr/src/boot contains the FreeBSD loader (ported to illumos) along with headers required for compatibility. usr/src/uts/common/sys/mkdev.h is likely where you want to start.

@jclulow
Copy link
Contributor

jclulow commented Nov 11, 2022

Those are not the major/minor/makdev macros for illumos itself. Although it's not advertised especially well, usr/src/boot contains the FreeBSD loader (ported to illumos) along with headers required for compatibility. usr/src/uts/common/sys/mkdev.h is likely where you want to start.

And, critically, makedev() and major() and minor() expand to function calls with a curried argument; e.g., makedev(a, b) becomes __makedev(1, a, b), as you can see in the disassembly of this program:

#include <stdlib.h>
#include <stdio.h>
#include <sys/mkdev.h>

int
main()
{
        dev_t dev = makedev(0x81, 0x82);

        printf("dev = %lx\n", dev);
}
$ gcc -m64 -Wall -Wextra -o md md.c

$ mdb -e 'main::dis' md
main:                           pushq  %rbp
main+1:                         movq   %rsp,%rbp
main+4:                         subq   $0x10,%rsp

main+8:                         movl   $0x82,%edx                     arg2
main+0xd:                       movl   $0x81,%esi                     arg1
main+0x12:                      movl   $0x1,%edi                      arg0
main+0x17:                      call   -0x1ae   <PLT:__makedev>       call __makedev()

main+0x1c:                      movq   %rax,-0x8(%rbp)
main+0x20:                      movq   -0x8(%rbp),%rax
main+0x24:                      movq   %rax,%rsi
main+0x27:                      movl   $0x40117c,%edi
main+0x2c:                      movl   $0x0,%eax
main+0x31:                      call   -0x1b8   <PLT:printf>
main+0x36:                      movl   $0x0,%eax
main+0x3b:                      leave
main+0x3c:                      ret

$ ./md
dev = 8100000082

@SteveLauC
Copy link
Contributor Author

Thank you both for showing me this!

I just took a look at usr/src/uts/common/sys/mkdev.h, and it seems that the functions themselves are implemented in usr/src/lib/libc/port/gen/mkdev.c.

So If we want them in Rust, is there any way we can do this? Thanks:)

@jclulow
Copy link
Contributor

jclulow commented Nov 11, 2022

Thank you both for showing me this!

You are most welcome!

I just took a look at usr/src/uts/common/sys/mkdev.h, and it seems that the functions themselves are implemented in usr/src/lib/libc/port/gen/mkdev.c.

That's correct. We implement them as functions, rather than as macros, so that we can make changes to their implementation without requiring people to recompile their applications.

So If we want them in Rust, is there any way we can do this? Thanks:)

Definitely! You'll need to create an extern "C" definition for the internal functions (__makedev(), etc) and then create a wrapper that has the documented signature just like the other platforms. That wrapper would call the internal __makedev() function with NEWDEV as the first argument, and then just pass on the arguments from the wrapper.

Something like:

extern "C" {
    fn __makedev(version: ::c_int, majdev: ::major_t, mindev: ::mindev) -> ::dev_t;
}

const NEWDEV: ::c_int = 1;

pub fn makedev(maj: ::major_t, min: ::mindev) -> ::dev_t {
    unsafe { __makedev(NEWDEV, maj, min) }
}

(I have not tried this myself)

@semarie
Copy link
Contributor

semarie commented Nov 11, 2022

I would use something like:

#[link_name="__makedev"]
extern "C" {
  pub fn makedev(maj: ::major_t, min: ::mindev) -> ::dev_t;
}

the name makedev will be used in Rust code, and linked to __makedev symbol in the binary.

@semarie
Copy link
Contributor

semarie commented Nov 11, 2022

ah, my bad. I didn't noticed that __makedev and makedev hasn't the same signature ! so the version from @jclulow is the one to use.

@semarie
Copy link
Contributor

semarie commented Nov 11, 2022

regarding the OpenBSD part, it LGTM.

@SteveLauC
Copy link
Contributor Author

So If we want them in Rust, is there any way we can do this? Thanks:)

Definitely! You'll need to create an extern "C" definition for the internal functions (__makedev(), etc) and then create a wrapper that has the documented signature just like the other platforms. That wrapper would call the internal __makedev() function with NEWDEV as the first argument, and then just pass on the arguments from the wrapper.

Something like:

extern "C" {
    fn __makedev(version: ::c_int, majdev: ::major_t, mindev: ::mindev) -> ::dev_t;
}

const NEWDEV: ::c_int = 1;

pub fn makedev(maj: ::major_t, min: ::mindev) -> ::dev_t {
    unsafe { __makedev(NEWDEV, maj, min) }
}

(I have not tried this myself)

Thanks! Let's link to it!

@SteveLauC
Copy link
Contributor Author

Friendly ping @jclulow @pfmooney.

Can I trouble you guys for another review of the illumos part of this PR:)

@bors
Copy link
Contributor

bors commented Dec 3, 2022

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

@JohnTitor
Copy link
Member

Ping again @jclulow @pfmooney, could you check this PR when you're available? Thanks!

@rustbot
Copy link
Collaborator

rustbot commented Apr 20, 2023

Some changes occurred in solarish module

cc @jclulow, @pfmooney

Some changes occurred in OpenBSD module

cc @semarie

@SteveLauC
Copy link
Contributor Author

SteveLauC commented Apr 20, 2023

Rebased this branch to the latest master:)

@pfmooney
Copy link
Contributor

Friendly ping @jclulow @pfmooney.

Can I trouble you guys for another review of the illumos part of this PR:)

Please add the necessary header requirement so that libc-test is appeased:

diff --git a/libc-test/build.rs b/libc-test/build.rs
index ac0f996fc4..8c20546771 100644
--- a/libc-test/build.rs
+++ b/libc-test/build.rs
@@ -821,6 +821,7 @@ fn test_solarish(target: &str) {
         "sys/ioctl.h",
         "sys/lgrp_user.h",
         "sys/loadavg.h",
+        "sys/mkdev.h",
         "sys/mman.h",
         "sys/mount.h",
         "sys/priv.h",

Otherwise the illumos bits look good to me.

Thanks

@SteveLauC
Copy link
Contributor Author

Friendly ping @jclulow @pfmooney.
Can I trouble you guys for another review of the illumos part of this PR:)

Please add the necessary header requirement so that libc-test is appeased:

diff --git a/libc-test/build.rs b/libc-test/build.rs
index ac0f996fc4..8c20546771 100644
--- a/libc-test/build.rs
+++ b/libc-test/build.rs
@@ -821,6 +821,7 @@ fn test_solarish(target: &str) {
         "sys/ioctl.h",
         "sys/lgrp_user.h",
         "sys/loadavg.h",
+        "sys/mkdev.h",
         "sys/mman.h",
         "sys/mount.h",
         "sys/priv.h",

Otherwise the illumos bits look good to me.

Thanks

Header added:)

@JohnTitor Let's give CI a try, if passed, then I will squash my commits

@JohnTitor
Copy link
Member

👍 @bors try

@bors
Copy link
Contributor

bors commented May 6, 2023

⌛ Trying commit f714e56 with merge e9e9c89...

bors added a commit that referenced this pull request May 6, 2023
add major/minor on BSDs/illumos

This PR adds `major/minor` on BSDs and `major/minor/makedev` on illumos.

Ref:
* [FreeBSD 11](https://github.com/freebsd/freebsd-src/blob/3e9337c6b211e778829ed3af783cd41447a8721b/sys/sys/types.h#L372-L374)
   ```c
   #define	major(x)	((int)(((u_int)(x) >> 8)&0xff))	/* major number */
   #define	minor(x)	((int)((x)&0xffff00ff))		/* minor number */
   ```
* [FreeBSD 12/13/14](https://github.com/freebsd/freebsd-src/blob/3d98e253febf816e6e2aea7d3b1c013c421895de/sys/sys/types.h#L332-L341)
  ```c
  static __inline int
  __major(dev_t _d)
  {
      return (((_d >> 32) & 0xffffff00) | ((_d >> 8) & 0xff));
  }

  static __inline int
  __minor(dev_t _d)
  {
      return (((_d >> 24) & 0xff00) | (_d & 0xffff00ff));
  }
  ```
* [DragonFly](https://github.com/DragonFlyBSD/DragonFlyBSD/blob/d7a10f947f5344fc95e874ca3b83e9e8d0986b25/sys/sys/types.h#L170-L171)
  ```c
  #define	major(x)	((int)(((u_int)(x) >> 8)&0xff)) /* major number */
  #define	minor(x)	((int)((x)&0xffff00ff))         /* minor number */
  ```
* [NetBSD](https://github.com/NetBSD/src/blob/a25a6fec1b0a676fc5b36fa01b2990e775775d90/sys/sys/types.h#L264-L266)
   ```c
  #define	major(x)	((devmajor_t)(((uint32_t)(x) & 0x000fff00) >>  8))
  #define	minor(x)	((devminor_t)((((uint32_t)(x) & 0xfff00000) >> 12) | \
				                      (((uint32_t)(x) & 0x000000ff) >>  0)))
   ```
* [OpenBSD](https://github.com/openbsd/src/blob/05cbc9aa8d8372e83274c75e35add6b8073c26f5/sys/sys/types.h#L211-L212)
   ```c
   #define	major(x)	(((unsigned)(x) >> 8) & 0xff)
   #define	minor(x)	((unsigned)((x) & 0xff) | (((x) & 0xffff0000) >> 8))
   ```
* illumos:

  1. [mkdev.c](https://github.com/illumos/illumos-gate/blob/8b26092d555bd1deaacf79ea64da374602aefb65/usr/src/lib/libc/port/gen/mkdev.c#L40-L146)
  2. [mkdev.h](https://github.com/illumos/illumos-gate/blob/8b26092d555bd1deaacf79ea64da374602aefb65/usr/src/uts/common/sys/mkdev.h#L97-L99)
@bors
Copy link
Contributor

bors commented May 6, 2023

💔 Test failed - checks-actions

@SteveLauC
Copy link
Contributor Author

Build Channels Linux (beta) failed due to an unnecessary parentheses, just have it fixed, let's try the CI again:)

@JohnTitor
Copy link
Member

@bors try

@bors
Copy link
Contributor

bors commented May 6, 2023

⌛ Trying commit 6c19c7c with merge f4cae67...

bors added a commit that referenced this pull request May 6, 2023
add major/minor on BSDs/illumos

This PR adds `major/minor` on BSDs and `major/minor/makedev` on illumos.

Ref:
* [FreeBSD 11](https://github.com/freebsd/freebsd-src/blob/3e9337c6b211e778829ed3af783cd41447a8721b/sys/sys/types.h#L372-L374)
   ```c
   #define	major(x)	((int)(((u_int)(x) >> 8)&0xff))	/* major number */
   #define	minor(x)	((int)((x)&0xffff00ff))		/* minor number */
   ```
* [FreeBSD 12/13/14](https://github.com/freebsd/freebsd-src/blob/3d98e253febf816e6e2aea7d3b1c013c421895de/sys/sys/types.h#L332-L341)
  ```c
  static __inline int
  __major(dev_t _d)
  {
      return (((_d >> 32) & 0xffffff00) | ((_d >> 8) & 0xff));
  }

  static __inline int
  __minor(dev_t _d)
  {
      return (((_d >> 24) & 0xff00) | (_d & 0xffff00ff));
  }
  ```
* [DragonFly](https://github.com/DragonFlyBSD/DragonFlyBSD/blob/d7a10f947f5344fc95e874ca3b83e9e8d0986b25/sys/sys/types.h#L170-L171)
  ```c
  #define	major(x)	((int)(((u_int)(x) >> 8)&0xff)) /* major number */
  #define	minor(x)	((int)((x)&0xffff00ff))         /* minor number */
  ```
* [NetBSD](https://github.com/NetBSD/src/blob/a25a6fec1b0a676fc5b36fa01b2990e775775d90/sys/sys/types.h#L264-L266)
   ```c
  #define	major(x)	((devmajor_t)(((uint32_t)(x) & 0x000fff00) >>  8))
  #define	minor(x)	((devminor_t)((((uint32_t)(x) & 0xfff00000) >> 12) | \
				                      (((uint32_t)(x) & 0x000000ff) >>  0)))
   ```
* [OpenBSD](https://github.com/openbsd/src/blob/05cbc9aa8d8372e83274c75e35add6b8073c26f5/sys/sys/types.h#L211-L212)
   ```c
   #define	major(x)	(((unsigned)(x) >> 8) & 0xff)
   #define	minor(x)	((unsigned)((x) & 0xff) | (((x) & 0xffff0000) >> 8))
   ```
* illumos:

  1. [mkdev.c](https://github.com/illumos/illumos-gate/blob/8b26092d555bd1deaacf79ea64da374602aefb65/usr/src/lib/libc/port/gen/mkdev.c#L40-L146)
  2. [mkdev.h](https://github.com/illumos/illumos-gate/blob/8b26092d555bd1deaacf79ea64da374602aefb65/usr/src/uts/common/sys/mkdev.h#L97-L99)
@bors
Copy link
Contributor

bors commented May 6, 2023

☀️ Try build successful - checks-actions, checks-cirrus-freebsd-12, checks-cirrus-freebsd-13, checks-cirrus-freebsd-14
Build commit: f4cae67 (f4cae6738d79a7f00011787b76ee5d82b4bd14b9)

@SteveLauC
Copy link
Contributor Author

Commits squashed, Let's merge it😆

@JohnTitor
Copy link
Member

Thanks! @bors r+

@bors
Copy link
Contributor

bors commented May 6, 2023

📌 Commit 0449416 has been approved by JohnTitor

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented May 6, 2023

⌛ Testing commit 0449416 with merge cdf9aa7...

@bors
Copy link
Contributor

bors commented May 6, 2023

☀️ Test successful - checks-actions, checks-cirrus-freebsd-12, checks-cirrus-freebsd-13, checks-cirrus-freebsd-14
Approved by: JohnTitor
Pushing cdf9aa7 to master...

@bors bors merged commit cdf9aa7 into rust-lang:master May 6, 2023
@SteveLauC SteveLauC deleted the major/minor-on-BSDs branch May 6, 2023 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants