On Thu, 3 Aug 2017, Konstantin Belousov wrote:
On Thu, Aug 03, 2017 at 01:21:56PM +1000, Bruce Evans wrote:
It would be better to remove the comments.
makedev() actually has a man page (a FreeBSD addition makedev(3)).
This could have been better, and is now out of date. The largest error
is that major() is still documented to return a value between 0 and
255. This reduction prevented makedev() being the inverse of
major()+minor() on arbitrary args. The man page doesn't claim that
it is, and barely gives a hint that makedev() can be used to synthesize
a dev_t from (x, y) provided 0 <= x <= 255.
See below.
diff --git a/share/man/man3/makedev.3 b/share/man/man3/makedev.3
index 87ca953dc79..a54d0590bc5 100644
--- a/share/man/man3/makedev.3
+++ b/share/man/man3/makedev.3
...
@@ -54,11 +54,18 @@ and
.Fn minor
macros can be used to obtain the original numbers from the device number
Problems with old wording:
- "can be used" is too informal. Before this, makedev() is described as
"allows" something. "allow" is almost never used in share/man3/*,
even in the sense of giving permission. Only pthread man pages have
many instances of this bug.
- "original" is an anachronism. The major and minor used to be the original
numbers, but now it is the dev_t that is original except when the dev_t
is synthesized. "original" might also mean "original representation".
Fixes: "can be used to obtain" is just a verbose spelling of "return".
"allows" can be improved using "returns" too, and it would be good to
shorten "a unique device number to be generated based on" to something
that says less about the one to one correspondence. "generation" is
just a special case of applying the correspondence to get an alternative
representation. "original" is also related to the correspondence.
Perhaps describe the correspondence first and then only say that the
functions are the ones that implement it.
.Fa dev .
+In other words, for a value
+.Va dev
+of the type
+.Vt dev_t ,
+the assertion
+.Dl dev == makedev(major(dev), minor(dev))
+is valid.
This describes the correspondence in another way. Pehaps delete all
details about what the functions do in ther previous description and
say that they are defined by this property. I think the inverse property
also needs to be spelled out. It is that for (int ma, int mi)
.Dl ma = major(makedev(ma, mi))
.Dl mi = minor(makedev(ma, mi))
.Pp
In previous implementations of
.Fx
all block and character devices were uniquely identified by a pair of
-major and minor numbers.
+stable major and minor numbers.
"stable" is clarified later, but is hard to understand in context.
The major number referred to a certain device class (e.g. disks, TTYs)
while the minor number identified an instance within the device class.
Later versions of
@@ -66,7 +73,8 @@ Later versions of
automatically generate a unique device number for each character device
visible in
.Pa /dev/ .
-These numbers are not divided in device classes.
+These numbers are not divided in device classes and are not guaranteed
+to be stable upon reboot or driver reload.
They used to be stable across even more than reboot. They were kept in
file systems, and that required them to be stable across OS versions in
practice.
.Pp
On
.Fx
@@ -78,11 +86,9 @@ conventional way.
.Sh RETURN VALUES
The
.Fn major
-macro returns a device major number that has a value between 0 and 255.
-The
+and
.Fn minor
-macro returns a device minor number whose value can span the complete
-range of an
+macros return numbers whose value can span the complete range of an
.Vt int .
.Sh SEE ALSO
.Xr mknod 2 ,
diff --git a/sys/sys/types.h b/sys/sys/types.h
index 30a08724443..8ea6e146e08 100644
--- a/sys/sys/types.h
+++ b/sys/sys/types.h
@@ -364,9 +364,9 @@ __bitcount64(__uint64_t _x)
#include <sys/select.h>
-#define major(x) ((int)((dev_t)(x) >> 32)) /* major number */
-#define minor(x) ((int)((x) & 0xffffffff)) /* minor number */
-#define makedev(x, y) (((dev_t)(x) << 32) | (unsigned)(y)) /* create
dev_t */
+#define major(x) ((int)((dev_t)(x) >> 32))
+#define minor(x) ((int)((x) & 0xffffffff))
+#define makedev(x, y) (((dev_t)(x) << 32) | (unsigned)(y))
I see another problem. Masking with 0xfffffffff and casting to unsigned
are gratuitously different spellings for extracting the low 32 bits.
I prefer the cast. It only works if ints have 32 bits, but we assume
that for other reasons. The mask is especially hard to understand for
the 1's complement case. We avoid complications with dodgy args for
major() by casting the arg to dev_t. For minor(), the behaviour is
only clear if x is unsigned (it should be dev_t), or int or lower rank
unsigned and typeof(0xffffffff) == unsigned (true with 32-bit ints).
In the second case, the default promotions convert x to unsigned. -0
becomes 0U which probably breaks it on 1's complement systems, but -0L
doesn't change, which is hard to understand. To avoid complications,
we could cast to dev_t, but that is verbose, or to unsigned, but then
the mask is redundant if ints are 32 bits. For makedev() we, do just
cast to unsigned and assume 32 bits.
/*
* These declarations belong elsewhere, but are repeated here and in
Bruce
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
