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

Port pkg/system/mknod.go to FreeBSD #42866

Merged

Conversation

akhramov
Copy link
Contributor

@akhramov akhramov commented Sep 19, 2021

fixes #42849

Because FreeBSD uses 64-bit device nodes (see
https://reviews.freebsd.org/rS318736), Linux implementation of
system.Mknod & system.Mkdev is not sufficient.

This change adds freebsd-specific implementations for Mknod and
Mkdev.

@akhramov akhramov force-pushed the feature/pkg-system-port-to-FreeBSD branch from 61cc0f3 to 3d6723c Compare September 19, 2021 11:09
Comment on lines 18 to 19
func Mkdev(major int64, minor int64) uint64 {
return unix.Mkdev(uint32(major), uint32(minor))
Copy link
Member

Choose a reason for hiding this comment

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

Doing a quick diff between both, to see what the difference is. On Linux we're converting this to return an uint32. All places where it's used (currently) cast it to an int (as argument for Mknod).

Mostly wondering if we should do the same for freebsd so that the signature is the same. (Perhaps we should look at that in a follow-up, and move these utilities into pkg/archive).

@samuelkarp perhaps you have thoughts on this?

diff --git a/pkg/system/mknod.go b/pkg/system/mknod_freebsd.go
index ccb2e638bb..b1ba8320dd 100644
--- a/pkg/system/mknod.go
+++ b/pkg/system/mknod_freebsd.go
@@ -1,5 +1,5 @@
-//go:build !freebsd && !windows
-// +build !freebsd,!windows
+//go:build freebsd
+// +build freebsd

 package system // import "github.com/docker/docker/pkg/system"

@@ -10,14 +10,11 @@ import (
 // Mknod creates a filesystem node (file, device special file or named pipe) named path
 // with attributes specified by mode and dev.
 func Mknod(path string, mode uint32, dev int) error {
-       return unix.Mknod(path, mode, dev)
+       return unix.Mknod(path, mode, uint64(dev))
 }

-// Mkdev is used to build the value of linux devices (in /dev/) which specifies major
+// Mkdev is used to build the value of FreeBSD devices (in /dev/) which specifies major
 // and minor number of the newly created device special file.
-// Linux device nodes are a bit weird due to backwards compat with 16 bit device nodes.
-// They are, from low to high: the lower 8 bits of the minor, then 12 bits of the major,
-// then the top 12 bits of the minor.
-func Mkdev(major int64, minor int64) uint32 {
-       return uint32(unix.Mkdev(uint32(major), uint32(minor)))
+func Mkdev(major int64, minor int64) uint64 {
+       return unix.Mkdev(uint32(major), uint32(minor))
 }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, thanks

Because FreeBSD uses 64-bit device nodes (see
https://reviews.freebsd.org/rS318736), Linux implementation of
`system.Mknod` & `system.Mkdev` is not sufficient.

This change adds freebsd-specific implementations for `Mknod` and
Mkdev`.

Signed-off-by: Artem Khramov <[email protected]>
@akhramov akhramov force-pushed the feature/pkg-system-port-to-FreeBSD branch from 3d6723c to f3d3994 Compare September 22, 2021 06:48
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@thaJeztah
Copy link
Member

@samuelkarp @AkihiroSuda ptal

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.

Can't build github.com/docker/docker/pkg/system on freebsd
3 participants