Re: svn commit: r321920 - head/sys/sys

2017-08-06 Thread Bruce Evans 

On Sat, 5 Aug 2017, Konstantin Belousov wrote:


On Sat, Aug 05, 2017 at 12:26:03PM +1000, Bruce Evans wrote:

+#defineminor(x)((int)(x))


Another nice simplification.  Strictly, it should be (int)(dev_t)(x) since
the pseudo-prototype says that the arg is converted to dev_t, but yesterday
I couldn't see any differences even for exotic x and exotic arches.

Today I can see some difference for exotic x and perverse implementations.
E.g., x with extra bits in a large signed integer type to invoke
implementation-defined behaviour, and a perverse implementation that
truncates to 0 for direct conversion to int but does the right thing
for indirect conversion).  But we depend on the implementation doing
the right thing for other casts to int.

Also, if dev_t == uintptr_t, it is valid for the caller to keep dev_t's
in void *'s internally but not to pass void * to minor() or major()
according to the prototype.  However, casting in the macros breaks the
warning for this.  I think pure macros should not cast their args or
pretend to have prototypes, but require callers to pass args of supported
types.  The old dev_t macros were closer to this -- they have expressions
like ((x) << 8) which will fail if x is not an integral type or give
wrong results ix x has extra bits.

So you are arguing to keep the & 0x operation ?


No, I don't want to go that deep.


I think this is not needed, since your note about cast being equivalent
holds for all supported architectures and I do not see anywhere a contract
for the operations to work on non-dev_t.


The pseudo-prototypes look like they provide such a contract.  They must be
read sort of backwards as saying that it is the caller's responsibility
to pass args of the type in the pseudo-prototype, like for a K function
or a C90 function with prototype is in scope.


So are you fine with the posted patch to sys/types.h ?


OK.

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"

Re: svn commit: r321920 - head/sys/sys

2017-08-05 Thread Konstantin Belousov 
On Sat, Aug 05, 2017 at 12:26:03PM +1000, Bruce Evans wrote:
> > +#defineminor(x)((int)(x))
> 
> Another nice simplification.  Strictly, it should be (int)(dev_t)(x) since
> the pseudo-prototype says that the arg is converted to dev_t, but yesterday
> I couldn't see any differences even for exotic x and exotic arches.
> 
> Today I can see some difference for exotic x and perverse implementations.
> E.g., x with extra bits in a large signed integer type to invoke
> implementation-defined behaviour, and a perverse implementation that
> truncates to 0 for direct conversion to int but does the right thing
> for indirect conversion).  But we depend on the implementation doing
> the right thing for other casts to int.
> 
> Also, if dev_t == uintptr_t, it is valid for the caller to keep dev_t's
> in void *'s internally but not to pass void * to minor() or major()
> according to the prototype.  However, casting in the macros breaks the
> warning for this.  I think pure macros should not cast their args or
> pretend to have prototypes, but require callers to pass args of supported
> types.  The old dev_t macros were closer to this -- they have expressions
> like ((x) << 8) which will fail if x is not an integral type or give
> wrong results ix x has extra bits.
So you are arguing to keep the & 0x operation ?
I think this is not needed, since your note about cast being equivalent
holds for all supported architectures and I do not see anywhere a contract
for the operations to work on non-dev_t.

So are you fine with the posted patch to sys/types.h ?

___
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"

Re: svn commit: r321920 - head/sys/sys

2017-08-04 Thread Bruce Evans 

On Thu, 3 Aug 2017, Konstantin Belousov wrote:


On Thu, Aug 03, 2017 at 07:34:56PM +1000, Bruce Evans wrote:

I see another problem.  Masking with 0xf and casting to unsigned
are gratuitously different spellings for extracting the low 32 bits.
I prefer the cast.


Below is one more update.  I reformulated the man page text, but not too
deep.  Also I replaced masking with the cast.


OK.


diff --git a/sys/sys/types.h b/sys/sys/types.h
index 30a08724443..eacbc1ba0c4 100644
--- a/sys/sys/types.h
+++ b/sys/sys/types.h
@@ -364,9 +364,9 @@ __bitcount64(__uint64_t _x)

#include 

-#definemajor(x)((int)((dev_t)(x) >> 32)) /* major number */
-#defineminor(x)((int)((x) & 0x))   /* minor number */
-#definemakedev(x, y)   (((dev_t)(x) << 32) | (unsigned)(y)) /* create 
dev_t */
+#definemajor(x)((int)((dev_t)(x) >> 32))
+#defineminor(x)((int)(x))


Another nice simplification.  Strictly, it should be (int)(dev_t)(x) since
the pseudo-prototype says that the arg is converted to dev_t, but yesterday
I couldn't see any differences even for exotic x and exotic arches.

Today I can see some difference for exotic x and perverse implementations.
E.g., x with extra bits in a large signed integer type to invoke
implementation-defined behaviour, and a perverse implementation that
truncates to 0 for direct conversion to int but does the right thing
for indirect conversion).  But we depend on the implementation doing
the right thing for other casts to int.

Also, if dev_t == uintptr_t, it is valid for the caller to keep dev_t's
in void *'s internally but not to pass void * to minor() or major()
according to the prototype.  However, casting in the macros breaks the
warning for this.  I think pure macros should not cast their args or
pretend to have prototypes, but require callers to pass args of supported
types.  The old dev_t macros were closer to this -- they have expressions
like ((x) << 8) which will fail if x is not an integral type or give
wrong results ix x has extra bits.

POSIX had to fix related problems for the ntohl() family.  The result
seems to be that the APIs act as if they are functions.  So even if
they are implemented as macros, the macros can't just cast their args,
but must be wrapped by functions (which can be inline) to get less
forceful conversions.  FreeBSD's implementation does almost exactly
this.  It has to cast args for optimizing const args, but seems to get
this right by not casting in the non-const case.  However, I have
uncommitted "fixes" which do cast in the non-const case.  Apparently
my fixes are backwards.


+#definemakedev(x, y)   (((dev_t)(x) << 32) | (unsigned)(y))

/*
 * 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"

Re: svn commit: r321920 - head/sys/sys

2017-08-03 Thread Konstantin Belousov 
On Thu, Aug 03, 2017 at 07:34:56PM +1000, Bruce Evans wrote:
> I see another problem.  Masking with 0xf and casting to unsigned
> are gratuitously different spellings for extracting the low 32 bits.
> I prefer the cast.

Below is one more update.  I reformulated the man page text, but not too
deep.  Also I replaced masking with the cast.

diff --git a/share/man/man3/makedev.3 b/share/man/man3/makedev.3
index 87ca953dc79..8ce9c554a09 100644
--- a/share/man/man3/makedev.3
+++ b/share/man/man3/makedev.3
@@ -24,7 +24,7 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd September 28, 2008
+.Dd August 3, 2017
 .Dt MAKEDEV 3
 .Os
 .Sh NAME
@@ -43,7 +43,7 @@
 .Sh DESCRIPTION
 The
 .Fn makedev
-macro allows a unique device number to be generated based on its
+macro returns a device number created from the provided
 .Fa major
 and
 .Fa minor
@@ -52,13 +52,26 @@ The
 .Fn major
 and
 .Fn minor
-macros can be used to obtain the original numbers from the device number
+macros return the original numbers from the device number
 .Fa dev .
+In other words, for a value
+.Va dev
+of the type
+.Vt dev_t ,
+and values
+.Va ma , mi
+of the type
+.Vt int ,
+the assertions
+.Dl dev == makedev(major(dev), minor(dev))
+.Dl ma == major(makedev(ma, mi))
+.Dl mi == minor(makedev(ma, mi))
+are valid.
 .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.
 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 +79,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.
 .Pp
 On
 .Fx
@@ -78,11 +92,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..eacbc1ba0c4 100644
--- a/sys/sys/types.h
+++ b/sys/sys/types.h
@@ -364,9 +364,9 @@ __bitcount64(__uint64_t _x)
 
 #include 
 
-#definemajor(x)((int)((dev_t)(x) >> 32))   /* major number 
*/
-#defineminor(x)((int)((x) & 0x))   /* minor number 
*/
-#definemakedev(x, y)   (((dev_t)(x) << 32) | (unsigned)(y)) /* create 
dev_t */
+#definemajor(x)((int)((dev_t)(x) >> 32))
+#defineminor(x)((int)(x))
+#definemakedev(x, y)   (((dev_t)(x) << 32) | (unsigned)(y))
 
 /*
  * These declarations belong elsewhere, but are repeated here and in
___
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"

Re: svn commit: r321920 - head/sys/sys

2017-08-03 Thread Bruce Evans 

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 

-#definemajor(x)((int)((dev_t)(x) >> 32)) /* major number */
-#defineminor(x)((int)((x) & 0x))   /* minor number */
-#definemakedev(x, y)   (((dev_t)(x) << 32) | (unsigned)(y)) /* create 
dev_t */
+#definemajor(x)((int)((dev_t)(x) >> 32))
+#defineminor(x)((int)((x) & 0x))
+#definemakedev(x, y)   (((dev_t)(x) << 32) | (unsigned)(y))


I see another problem.  Masking with 0xf 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(0x) == 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

Re: svn commit: r321920 - head/sys/sys

2017-08-03 Thread Konstantin Belousov 
On Thu, Aug 03, 2017 at 10:02:48AM +0200, Hans Petter Selasky wrote:
> On 08/03/17 09:57, Konstantin Belousov wrote:
> >   .Xr mknod 2 ,
> 
> Should mknod be removed from base or stubbed in light of the more recent 
> devfs ?

mknod is used on devfs as a way to 'undelete' devfs node.  You may do
rm /dev/node
and then
mknod /dev/node ...
would restore it.

Also, for UFS or other filesystems which are exported by NFS, mknod is
useful to provide device nodes for non-FreeBSD clients.
___
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"

Re: svn commit: r321920 - head/sys/sys

2017-08-03 Thread Hans Petter Selasky 

On 08/03/17 09:57, Konstantin Belousov wrote:

  .Xr mknod 2 ,


Should mknod be removed from base or stubbed in light of the more recent 
devfs ?


--HPS
___
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"

Re: svn commit: r321920 - head/sys/sys

2017-08-03 Thread Konstantin Belousov 
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
@@ -24,7 +24,7 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd September 28, 2008
+.Dd August 3, 2017
 .Dt MAKEDEV 3
 .Os
 .Sh NAME
@@ -54,11 +54,18 @@ and
 .Fn minor
 macros can be used to obtain the original numbers from the device number
 .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.
 .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.
 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.
 .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 
 
-#definemajor(x)((int)((dev_t)(x) >> 32))   /* major number 
*/
-#defineminor(x)((int)((x) & 0x))   /* minor number 
*/
-#definemakedev(x, y)   (((dev_t)(x) << 32) | (unsigned)(y)) /* create 
dev_t */
+#definemajor(x)((int)((dev_t)(x) >> 32))
+#defineminor(x)((int)((x) & 0x))
+#definemakedev(x, y)   (((dev_t)(x) << 32) | (unsigned)(y))
 
 /*
  * These declarations belong elsewhere, but are repeated here and in
___
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"

Re: svn commit: r321920 - head/sys/sys

2017-08-02 Thread Bruce Evans 

On Wed, 2 Aug 2017, Konstantin Belousov wrote:


On Wed, Aug 02, 2017 at 02:38:50PM +0200, Hans Petter Selasky wrote:

On 08/02/17 14:36, Hans Petter Selasky wrote:

On 08/02/17 12:14, Konstantin Belousov wrote:

+#definemajor(x)((int)((dev_t)(x) >> 32))/* major number */
+#defineminor(x)((int)((x) & 0x))/* minor number */
+#definemakedev(x, y)(((dev_t)(x) << 32) | (y))/* create
dev_t */


One more comment on this issue:

I think makedev(x, y) should be declared like this, to avoid issues when
"y" is negative:

#definemakedev(x, y)(((dev_t)(x) << 32) | (unsigned int)(y))
/* create dev_t */


I think this has a lot of style bugs:
- long line constructed by blind expansion
- verbose spelling of 'unsigned' in the expansion to make the long line longer
- banal comment to make the long line.  The comment on this line is not quite
  as banal as the ones on the preceding 2 line -- those do less than echo the
  code.


And you'll probably want a final wrapping dev_t cast aswell. 128-bit
numbers are not yet there.

#define makedev(x, y)((dev_t)(((dev_t)(x) << 32) | (unsigned
int)(y)))
> /* create dev_t */


You mean 128-bit ints.


I agree with the usefulness of the y cast to unsigned type, but I am not sure
what is the use of final dev_t cast.  By the usual arithmetic conversion
rules, the final type of the '|' is the highest rank type of the operands.
Something unusual can only happen if int is wider than dev_t.


So it happens with 128 ints (or 65-bit ints) if dev_t remains 64 bits.


So I am going to commit the following update.


Almost OK.  The necessary casts and parentheses are already ugly enough.
This is the implementation, so it can assume that int is 32 bits and dev_t
is 64 bits and not have the complexity to support the general case.  Ints
are probably assumed to be 32 bits in a few thousand similar definitions
and a few million lines of code just in FreeBSD sources.


diff --git a/sys/sys/types.h b/sys/sys/types.h
index fce57e412ed..30a08724443 100644
--- a/sys/sys/types.h
+++ b/sys/sys/types.h
@@ -366,7 +366,7 @@ __bitcount64(__uint64_t _x)

#define major(x)((int)((dev_t)(x) >> 32)) /* major number */
#define minor(x)((int)((x) & 0x))   /* minor number */
-#definemakedev(x, y)   (((dev_t)(x) << 32) | (y))/* create 
dev_t */
+#definemakedev(x, y)   (((dev_t)(x) << 32) | (unsigned)(y)) /* create 
dev_t */

/*
 * These declarations belong elsewhere, but are repeated here and in


You fixed the long line by not spelling 'unsigned' verbosely and not
aligning the comments uniformly.  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.

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"

Re: svn commit: r321920 - head/sys/sys

2017-08-02 Thread Hans Petter Selasky 

On 08/02/17 15:54, Konstantin Belousov wrote:

I agree with the usefulness of the y cast to unsigned type, but I am not sure
what is the use of final dev_t cast.  By the usual arithmetic conversion
rules, the final type of the '|' is the highest rank type of the operands.
Something unusual can only happen if int is wider than dev_t.

So I am going to commit the following update.


OK, Looks good!

--HPS
___
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"

Re: svn commit: r321920 - head/sys/sys

2017-08-02 Thread Konstantin Belousov 
On Wed, Aug 02, 2017 at 02:38:50PM +0200, Hans Petter Selasky wrote:
> On 08/02/17 14:36, Hans Petter Selasky wrote:
> > On 08/02/17 12:14, Konstantin Belousov wrote:
> >> +#definemajor(x)((int)((dev_t)(x) >> 32))/* major number */
> >> +#defineminor(x)((int)((x) & 0x))/* minor number */
> >> +#definemakedev(x, y)(((dev_t)(x) << 32) | (y))/* create 
> >> dev_t */
> > 
> > One more comment on this issue:
> > 
> > I think makedev(x, y) should be declared like this, to avoid issues when 
> > "y" is negative:
> > 
> > #definemakedev(x, y)(((dev_t)(x) << 32) | (unsigned int)(y))
> > /* create dev_t */
> > 
> > ???
> > 
> > --HPS
> > 
> > 
> 
> And you'll probably want a final wrapping dev_t cast aswell. 128-bit 
> numbers are not yet there.
> 
> #define   makedev(x, y)((dev_t)(((dev_t)(x) << 32) | (unsigned 
> int)(y)))
>  > /* create dev_t */

I agree with the usefulness of the y cast to unsigned type, but I am not sure
what is the use of final dev_t cast.  By the usual arithmetic conversion 
rules, the final type of the '|' is the highest rank type of the operands.
Something unusual can only happen if int is wider than dev_t.

So I am going to commit the following update.

diff --git a/sys/sys/types.h b/sys/sys/types.h
index fce57e412ed..30a08724443 100644
--- a/sys/sys/types.h
+++ b/sys/sys/types.h
@@ -366,7 +366,7 @@ __bitcount64(__uint64_t _x)
 
 #definemajor(x)((int)((dev_t)(x) >> 32))   /* major number 
*/
 #defineminor(x)((int)((x) & 0x))   /* minor number 
*/
-#definemakedev(x, y)   (((dev_t)(x) << 32) | (y))  /* create dev_t 
*/
+#definemakedev(x, y)   (((dev_t)(x) << 32) | (unsigned)(y)) /* create 
dev_t */
 
 /*
  * These declarations belong elsewhere, but are repeated here and in
___
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"

Re: svn commit: r321920 - head/sys/sys

2017-08-02 Thread Hans Petter Selasky 

On 08/02/17 14:36, Hans Petter Selasky wrote:

On 08/02/17 12:14, Konstantin Belousov wrote:

+#definemajor(x)((int)((dev_t)(x) >> 32))/* major number */
+#defineminor(x)((int)((x) & 0x))/* minor number */
+#definemakedev(x, y)(((dev_t)(x) << 32) | (y))/* create 
dev_t */


One more comment on this issue:

I think makedev(x, y) should be declared like this, to avoid issues when 
"y" is negative:


#definemakedev(x, y)(((dev_t)(x) << 32) | (unsigned int)(y))
/* create dev_t */


???

--HPS




And you'll probably want a final wrapping dev_t cast aswell. 128-bit 
numbers are not yet there.


#define	makedev(x, y)((dev_t)(((dev_t)(x) << 32) | (unsigned 
int)(y)))

> /* create dev_t */

--HPS
___
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"

Re: svn commit: r321920 - head/sys/sys

2017-08-02 Thread Hans Petter Selasky 

On 08/02/17 14:37, Konstantin Belousov wrote:

si_drv1->xxx instead of using si_drv0 and dev2unit().

si_drv1 is 32bit on 32bit platforms.



I mean the LinuxKPI structure pointed to by si_drv1, not si_drv1 itself.

--HPS
___
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"

Re: svn commit: r321920 - head/sys/sys

2017-08-02 Thread Konstantin Belousov 
On Wed, Aug 02, 2017 at 02:26:46PM +0200, Hans Petter Selasky wrote:
> On 08/02/17 13:43, Konstantin Belousov wrote:
> > On Wed, Aug 02, 2017 at 01:27:50PM +0200, Hans Petter Selasky wrote:
> >> On 08/02/17 13:17, Konstantin Belousov wrote:
> >>> But y must be dev_t.
> >>
> >> Sure, but "struct cdev" 's si_drv0 is only "int" . How can it contain
> >> dev_t ?
> > 
> > Why should it contain dev_t ?
> > Linux KPI abused that field it seems.
> > 
> 
> Hi,
> 
> The LinuxKPI uses si_drv0 to contain the output from mkdev(). Before the 
> mkdev() function would fit into 32-bits. Now it is 64-bits.
> 
> I can fix the LinuxKPI bits this time. We can store this information in 
> si_drv1->xxx instead of using si_drv0 and dev2unit().
si_drv1 is 32bit on 32bit platforms.

> 
> > Lets change the focus of the discussion.
> > You cited the
> > struct linux_cdev *
> > linux_find_cdev(const char *name, unsigned major, unsigned minor)
> > function which finds cdev (or some mockup of the native cdev) by 
> > major/minor.
> > Where does these major/minor numbers come from ?
> 
> The major number is in the range 0..255 and decides the type of device.
> The minor number is in the range 0..255 aswell typically and defines a 
> unique number for each LinuxKPI created character device.
> 
> > I mean that if they are contructed as major(struct stat.st_rdev) and
> > minor(struct stat.st_rdev), then even the original code looks wrong
> > without the ino64 addition.  Since devfs reports the internal inode
> > number into st_rdev, which formally is not accessible outside the devfs
> > filesystem.  So should the code for linux_find_cdev() changed to match
> > cdevs against inode number ?
> 
> These numbers do not come from any user-visible files. They are just 
> used internally in the kernel to pass around information.
> 
> > cdp_inode is serially generated so on real machine it is really a small
> > number for any /dev node.  You can watch that by ls -l /dev.
> 
> Sure.
> 
> > 
> 
> While at it, I found that contrib/mknod/pack_dev.c, still has hardcoded 
> references to the old makedev():
> 
> > contrib/mknod/pack_dev.c:#definemakedev_freebsd(x,y)
> > ((portdev_t)x) << 8) & 0xff00) | \
> > contrib/mknod/pack_dev.c:   dev = makedev_freebsd(numbers[0], 
> > numbers[1]);
> 
> Same goes for: contrib/libarchive/libarchive/archive_pack_dev.c

I do not see how pack_dev.c can be used for anything non-broken on FreeBSD,
at least in connection with devfs.  For non-devfs filesystems it can
be streamlined, of course.

I will look at this later.

> 
> And if you "grep -r makedev /usr/src" you'll find more.
> 
> --HPS
___
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"

Re: svn commit: r321920 - head/sys/sys

2017-08-02 Thread Hans Petter Selasky 

On 08/02/17 12:14, Konstantin Belousov wrote:

+#definemajor(x)((int)((dev_t)(x) >> 32)) /* major number */
+#defineminor(x)((int)((x) & 0x))   /* minor number */
+#definemakedev(x, y)   (((dev_t)(x) << 32) | (y))/* create 
dev_t */


One more comment on this issue:

I think makedev(x, y) should be declared like this, to avoid issues when 
"y" is negative:


#define	makedev(x, y)	(((dev_t)(x) << 32) | (unsigned int)(y))	/* create 
dev_t */


???

--HPS
___
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"

Re: svn commit: r321920 - head/sys/sys

2017-08-02 Thread Hans Petter Selasky 

On 08/02/17 13:43, Konstantin Belousov wrote:

On Wed, Aug 02, 2017 at 01:27:50PM +0200, Hans Petter Selasky wrote:

On 08/02/17 13:17, Konstantin Belousov wrote:

But y must be dev_t.


Sure, but "struct cdev" 's si_drv0 is only "int" . How can it contain
dev_t ?


Why should it contain dev_t ?
Linux KPI abused that field it seems.



Hi,

The LinuxKPI uses si_drv0 to contain the output from mkdev(). Before the 
mkdev() function would fit into 32-bits. Now it is 64-bits.


I can fix the LinuxKPI bits this time. We can store this information in 
si_drv1->xxx instead of using si_drv0 and dev2unit().



Lets change the focus of the discussion.
You cited the
struct linux_cdev *
linux_find_cdev(const char *name, unsigned major, unsigned minor)
function which finds cdev (or some mockup of the native cdev) by major/minor.
Where does these major/minor numbers come from ?


The major number is in the range 0..255 and decides the type of device.
The minor number is in the range 0..255 aswell typically and defines a 
unique number for each LinuxKPI created character device.



I mean that if they are contructed as major(struct stat.st_rdev) and
minor(struct stat.st_rdev), then even the original code looks wrong
without the ino64 addition.  Since devfs reports the internal inode
number into st_rdev, which formally is not accessible outside the devfs
filesystem.  So should the code for linux_find_cdev() changed to match
cdevs against inode number ?


These numbers do not come from any user-visible files. They are just 
used internally in the kernel to pass around information.



cdp_inode is serially generated so on real machine it is really a small
number for any /dev node.  You can watch that by ls -l /dev.


Sure.





While at it, I found that contrib/mknod/pack_dev.c, still has hardcoded 
references to the old makedev():



contrib/mknod/pack_dev.c:#definemakedev_freebsd(x,y)((portdev_t)x) 
<< 8) & 0xff00) | \
contrib/mknod/pack_dev.c:   dev = makedev_freebsd(numbers[0], 
numbers[1]);


Same goes for: contrib/libarchive/libarchive/archive_pack_dev.c

And if you "grep -r makedev /usr/src" you'll find more.

--HPS
___
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"

Re: svn commit: r321920 - head/sys/sys

2017-08-02 Thread Konstantin Belousov 
On Wed, Aug 02, 2017 at 01:27:50PM +0200, Hans Petter Selasky wrote:
> On 08/02/17 13:17, Konstantin Belousov wrote:
> > But y must be dev_t.
> 
> Sure, but "struct cdev" 's si_drv0 is only "int" . How can it contain 
> dev_t ?

Why should it contain dev_t ?
Linux KPI abused that field it seems.

Lets change the focus of the discussion.
You cited the 
struct linux_cdev *
linux_find_cdev(const char *name, unsigned major, unsigned minor)
function which finds cdev (or some mockup of the native cdev) by major/minor.
Where does these major/minor numbers come from ?

I mean that if they are contructed as major(struct stat.st_rdev) and
minor(struct stat.st_rdev), then even the original code looks wrong
without the ino64 addition.  Since devfs reports the internal inode
number into st_rdev, which formally is not accessible outside the devfs
filesystem.  So should the code for linux_find_cdev() changed to match
cdevs against inode number ?

cdp_inode is serially generated so on real machine it is really a small
number for any /dev node.  You can watch that by ls -l /dev.
___
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"

Re: svn commit: r321920 - head/sys/sys

2017-08-02 Thread Hans Petter Selasky 

On 08/02/17 13:17, Konstantin Belousov wrote:

But y must be dev_t.


Sure, but "struct cdev" 's si_drv0 is only "int" . How can it contain 
dev_t ?


--HPS
___
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"

Re: svn commit: r321920 - head/sys/sys

2017-08-02 Thread Konstantin Belousov 
On Wed, Aug 02, 2017 at 01:11:48PM +0200, Hans Petter Selasky wrote:
> On 08/02/17 13:06, Konstantin Belousov wrote:
> > So linuxkpi was broken before this commit as well, as I noted in my
> > other reply ?
> 
> Hi,
> 
> The LinuxKPI uses minor/major/makedev internally.
> 
> y = makedev(z,t)
> 
> Then the LinuxKPI assumes z and t can be retrieved through minor and 
> major: minor(y) == z and major(y) == t
> 
> If "y" is an "int" and it is assigned a 64-bit value, it is obvious that 
> this code no longer holds true. Same might be in userspace/ports.
But y must be dev_t.

> 
> So yes, this change breaks the LinuxKPI, at least some parts of it.
> 
> --HPS
___
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"

Re: svn commit: r321920 - head/sys/sys

2017-08-02 Thread Hans Petter Selasky 

On 08/02/17 13:06, Konstantin Belousov wrote:

So linuxkpi was broken before this commit as well, as I noted in my
other reply ?


Hi,

The LinuxKPI uses minor/major/makedev internally.

y = makedev(z,t)

Then the LinuxKPI assumes z and t can be retrieved through minor and 
major: minor(y) == z and major(y) == t


If "y" is an "int" and it is assigned a 64-bit value, it is obvious that 
this code no longer holds true. Same might be in userspace/ports.


So yes, this change breaks the LinuxKPI, at least some parts of it.

--HPS
___
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"

Re: svn commit: r321920 - head/sys/sys

2017-08-02 Thread Konstantin Belousov 
On Wed, Aug 02, 2017 at 12:35:47PM +0200, Hans Petter Selasky wrote:
> On 08/02/17 12:14, Konstantin Belousov wrote:
> > Author: kib
> > Date: Wed Aug  2 10:14:17 2017
> > New Revision: 321920
> > URL: https://svnweb.freebsd.org/changeset/base/321920
> > 
> > Log:
> >Change major()/minor() to work with 64bit dev_t.
> >
> >Since traditional types for the macros values are int, remove the
> >cookie trick and just split the dev_t at the word boundary.
> >
> >Reported by: Victor Stinner 
> >PR:  221048
> >Sponsored by:The FreeBSD Foundation
> > 
> > Modified:
> >head/sys/sys/types.h
> > 
> > Modified: head/sys/sys/types.h
> > ==
> > --- head/sys/sys/types.hWed Aug  2 10:12:10 2017(r321919)
> > +++ head/sys/sys/types.hWed Aug  2 10:14:17 2017(r321920)
> > @@ -364,14 +364,9 @@ __bitcount64(__uint64_t _x)
> >   
> >   #include 
> >   
> > -/*
> > - * minor() gives a cookie instead of an index since we don't want to
> > - * change the meanings of bits 0-15 or waste time and space shifting
> > - * bits 16-31 for devices that don't use them.
> > - */
> > -#definemajor(x)((int)(((u_int)(x) >> 8)&0xff)) /* major number 
> > */
> > -#defineminor(x)((int)((x)&0x00ff)) /* minor number 
> > */
> > -#definemakedev(x,y)((dev_t)(((x) << 8) | (y))) /* create dev_t 
> > */
> > +#definemajor(x)((int)((dev_t)(x) >> 32))   /* major number 
> > */
> > +#defineminor(x)((int)((x) & 0x))   /* minor number 
> > */
> > +#definemakedev(x, y)   (((dev_t)(x) << 32) | (y))  /* create dev_t 
> > */
> >   
> >   /*
> >* These declarations belong elsewhere, but are repeated here and in
> > 
> 
> Hi,
> 
> This change breaks the LinuxKPI:
> 
> > struct linux_cdev *
> > linux_find_cdev(const char *name, unsigned major, unsigned minor)
> > {
> > int unit = MKDEV(major, minor);
> > struct cdev *cdev;
> >   
> > dev_lock();
> > LIST_FOREACH(cdev, _devs, si_list) {
> > struct linux_cdev *ldev = cdev->si_drv1;
> > if (dev2unit(cdev) == unit &&
> > strcmp(kobject_name(>kobj), name) == 0) {
> > break;
> > }
> > }
> > dev_unlock();
> >  
> > return (cdev != NULL ? cdev->si_drv1 : NULL);
> > }
So linuxkpi was broken before this commit as well, as I noted in my
other reply ?

> 
> You will also need to change cdev->si_drv0 to dev_t  and fix the 
> make_dev() functions too.
Why do you think that si_drv0 needs to be changes ?

> 
> > struct cdev {
> > void*si_spare0;
> > u_int   si_flags;
> > #define SI_ETERNAL  0x0001  /* never destroyed */
> > #define SI_ALIAS0x0002  /* carrier of alias name */
> > #define SI_NAMED0x0004  /* make_dev{_alias} has been called */
> > #define SI_CHEAPCLONE   0x0008  /* can be removed_dev'ed when vnode 
> > reclaims */
> > #define SI_CHILD0x0010  /* child of another struct cdev **/
> > #define SI_DUMPDEV  0x0080  /* is kernel dumpdev */
> > #define SI_CLONELIST0x0200  /* on a clone list */
> > #define SI_UNMAPPED 0x0400  /* can handle unmapped I/O */
> > #define SI_NOSPLIT  0x0800  /* I/O should not be split up */
> > struct timespec si_atime;
> > struct timespec si_ctime;
> > struct timespec si_mtime;
> > uid_t   si_uid;
> > gid_t   si_gid;
> > mode_t  si_mode;
> > struct ucred*si_cred;   /* cached clone-time credential */
> > int si_drv0;
> 
> 
> Further this update assigns a 64-bit value to tfsid.val[0], which is 
> 32-bit ??
> 
>  > sys/sys/mount.h:typedef struct fsid { int32_t val[2]; } fsid_t;/* 
> filesystem id type */
> 
> sys/kern/vfs_subr.c:tfsid.val[0] = makedev(255,
>From my reading if the vfs_getnewfsid() code, it is fine.  It ignores
the 255-major value, which does not add much sense to the returned value,
just that byte 1 is not zero and not 0xff.

> 
> And there are more places like this!
I see the use of makedev() in nfsserver, where the change seemingly improve
the situation by initializing na_rdev to the full 64 bit value, and in
fdescfs, which does not care.

> 
> Did you properly check all uses of makedev() in the kernel before making 
> this change?
> 
> --HPS
___
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"

Re: svn commit: r321920 - head/sys/sys

2017-08-02 Thread Konstantin Belousov 
On Wed, Aug 02, 2017 at 12:23:22PM +0200, Hans Petter Selasky wrote:
> On 08/02/17 12:14, Konstantin Belousov wrote:
> > Author: kib
> > Date: Wed Aug  2 10:14:17 2017
> > New Revision: 321920
> > URL: https://svnweb.freebsd.org/changeset/base/321920
> > 
> > Log:
> >Change major()/minor() to work with 64bit dev_t.
> >
> >Since traditional types for the macros values are int, remove the
> >cookie trick and just split the dev_t at the word boundary.
> >
> >Reported by: Victor Stinner 
> >PR:  221048
> >Sponsored by:The FreeBSD Foundation
> > 
> > Modified:
> >head/sys/sys/types.h
> > 
> > Modified: head/sys/sys/types.h
> > ==
> > --- head/sys/sys/types.hWed Aug  2 10:12:10 2017(r321919)
> > +++ head/sys/sys/types.hWed Aug  2 10:14:17 2017(r321920)
> > @@ -364,14 +364,9 @@ __bitcount64(__uint64_t _x)
> >   
> >   #include 
> >   
> > -/*
> > - * minor() gives a cookie instead of an index since we don't want to
> > - * change the meanings of bits 0-15 or waste time and space shifting
> > - * bits 16-31 for devices that don't use them.
> > - */
> > -#definemajor(x)((int)(((u_int)(x) >> 8)&0xff)) /* major number 
> > */
> > -#defineminor(x)((int)((x)&0x00ff)) /* minor number 
> > */
> > -#definemakedev(x,y)((dev_t)(((x) << 8) | (y))) /* create dev_t 
> > */
> > +#definemajor(x)((int)((dev_t)(x) >> 32))   /* major number 
> > */
> > +#defineminor(x)((int)((x) & 0x))   /* minor number 
> > */
> > +#definemakedev(x, y)   (((dev_t)(x) << 32) | (y))  /* create dev_t 
> > */
> >   
> >   /*
> >* These declarations belong elsewhere, but are repeated here and in
> 
> Hi,
> 
> This change looks like it affects user-space applications? Why is not 
> the FreeBSD version number bumped?

This change is the trailing fix for the ino64 commit.

More details are following: main feature of major/minor is the
deconstruction of the dev_t into components which can be used to
reconstruct original dev_t with makedev().  In other words, if
makedev(major(x), minor(x)) != x
then the major/minor API is useless.

And it was useless right after ino64 commit r318736 and up to the
commit you replied to.  The fact that nobody complained until the
referenced PR, indicates that the API is not actively used (finally).

I do not see a need to bump the __FreeBSD_version for this, we do not
support older HEADs in any way.  This means that we assume that HEAD
users always use the latest HEAD.  So no bump for such minor fix.
___
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"

Re: svn commit: r321920 - head/sys/sys

2017-08-02 Thread Hans Petter Selasky 

On 08/02/17 12:14, Konstantin Belousov wrote:

Author: kib
Date: Wed Aug  2 10:14:17 2017
New Revision: 321920
URL: https://svnweb.freebsd.org/changeset/base/321920

Log:
   Change major()/minor() to work with 64bit dev_t.
   
   Since traditional types for the macros values are int, remove the

   cookie trick and just split the dev_t at the word boundary.
   
   Reported by:	Victor Stinner 

   PR:  221048
   Sponsored by:The FreeBSD Foundation

Modified:
   head/sys/sys/types.h

Modified: head/sys/sys/types.h
==
--- head/sys/sys/types.hWed Aug  2 10:12:10 2017(r321919)
+++ head/sys/sys/types.hWed Aug  2 10:14:17 2017(r321920)
@@ -364,14 +364,9 @@ __bitcount64(__uint64_t _x)
  
  #include 
  
-/*

- * minor() gives a cookie instead of an index since we don't want to
- * change the meanings of bits 0-15 or waste time and space shifting
- * bits 16-31 for devices that don't use them.
- */
-#definemajor(x)((int)(((u_int)(x) >> 8)&0xff))   /* major 
number */
-#defineminor(x)((int)((x)&0x00ff)) /* minor 
number */
-#definemakedev(x,y)((dev_t)(((x) << 8) | (y)))   /* create 
dev_t */
+#definemajor(x)((int)((dev_t)(x) >> 32)) /* major number */
+#defineminor(x)((int)((x) & 0x))   /* minor number */
+#definemakedev(x, y)   (((dev_t)(x) << 32) | (y))/* create 
dev_t */
  
  /*

   * These declarations belong elsewhere, but are repeated here and in



Hi,

This change breaks the LinuxKPI:


struct linux_cdev *
linux_find_cdev(const char *name, unsigned major, unsigned minor)
{
int unit = MKDEV(major, minor);
struct cdev *cdev;
  
dev_lock();

LIST_FOREACH(cdev, _devs, si_list) {
struct linux_cdev *ldev = cdev->si_drv1;
if (dev2unit(cdev) == unit &&
strcmp(kobject_name(>kobj), name) == 0) {
break;
}
}
dev_unlock();
 
return (cdev != NULL ? cdev->si_drv1 : NULL);

}


You will also need to change cdev->si_drv0 to dev_t  and fix the 
make_dev() functions too.



struct cdev {
void*si_spare0;
u_int   si_flags;
#define SI_ETERNAL  0x0001  /* never destroyed */
#define SI_ALIAS0x0002  /* carrier of alias name */
#define SI_NAMED0x0004  /* make_dev{_alias} has been called */
#define SI_CHEAPCLONE   0x0008  /* can be removed_dev'ed when vnode reclaims */
#define SI_CHILD0x0010  /* child of another struct cdev **/
#define SI_DUMPDEV  0x0080  /* is kernel dumpdev */
#define SI_CLONELIST0x0200  /* on a clone list */
#define SI_UNMAPPED 0x0400  /* can handle unmapped I/O */
#define SI_NOSPLIT  0x0800  /* I/O should not be split up */
struct timespec si_atime;
struct timespec si_ctime;
struct timespec si_mtime;
uid_t   si_uid;
gid_t   si_gid;
mode_t  si_mode;
struct ucred*si_cred;   /* cached clone-time credential */
int si_drv0;



Further this update assigns a 64-bit value to tfsid.val[0], which is 
32-bit ??


> sys/sys/mount.h:typedef struct fsid { int32_t val[2]; } fsid_t;	/* 
filesystem id type */


sys/kern/vfs_subr.c:tfsid.val[0] = makedev(255,

And there are more places like this!

Did you properly check all uses of makedev() in the kernel before making 
this change?


--HPS
___
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"

Re: svn commit: r321920 - head/sys/sys

2017-08-02 Thread Hans Petter Selasky 

On 08/02/17 12:14, Konstantin Belousov wrote:

Author: kib
Date: Wed Aug  2 10:14:17 2017
New Revision: 321920
URL: https://svnweb.freebsd.org/changeset/base/321920

Log:
   Change major()/minor() to work with 64bit dev_t.
   
   Since traditional types for the macros values are int, remove the

   cookie trick and just split the dev_t at the word boundary.
   
   Reported by:	Victor Stinner 

   PR:  221048
   Sponsored by:The FreeBSD Foundation

Modified:
   head/sys/sys/types.h

Modified: head/sys/sys/types.h
==
--- head/sys/sys/types.hWed Aug  2 10:12:10 2017(r321919)
+++ head/sys/sys/types.hWed Aug  2 10:14:17 2017(r321920)
@@ -364,14 +364,9 @@ __bitcount64(__uint64_t _x)
  
  #include 
  
-/*

- * minor() gives a cookie instead of an index since we don't want to
- * change the meanings of bits 0-15 or waste time and space shifting
- * bits 16-31 for devices that don't use them.
- */
-#definemajor(x)((int)(((u_int)(x) >> 8)&0xff))   /* major 
number */
-#defineminor(x)((int)((x)&0x00ff)) /* minor 
number */
-#definemakedev(x,y)((dev_t)(((x) << 8) | (y)))   /* create 
dev_t */
+#definemajor(x)((int)((dev_t)(x) >> 32)) /* major number */
+#defineminor(x)((int)((x) & 0x))   /* minor number */
+#definemakedev(x, y)   (((dev_t)(x) << 32) | (y))/* create 
dev_t */
  
  /*

   * These declarations belong elsewhere, but are repeated here and in


Hi,

This change looks like it affects user-space applications? Why is not 
the FreeBSD version number bumped?


--HPS

___
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"

svn commit: r321920 - head/sys/sys

2017-08-02 Thread Konstantin Belousov 
Author: kib
Date: Wed Aug  2 10:14:17 2017
New Revision: 321920
URL: https://svnweb.freebsd.org/changeset/base/321920

Log:
  Change major()/minor() to work with 64bit dev_t.
  
  Since traditional types for the macros values are int, remove the
  cookie trick and just split the dev_t at the word boundary.
  
  Reported by:  Victor Stinner 
  PR:   221048
  Sponsored by: The FreeBSD Foundation

Modified:
  head/sys/sys/types.h

Modified: head/sys/sys/types.h
==
--- head/sys/sys/types.hWed Aug  2 10:12:10 2017(r321919)
+++ head/sys/sys/types.hWed Aug  2 10:14:17 2017(r321920)
@@ -364,14 +364,9 @@ __bitcount64(__uint64_t _x)
 
 #include 
 
-/*
- * minor() gives a cookie instead of an index since we don't want to
- * change the meanings of bits 0-15 or waste time and space shifting
- * bits 16-31 for devices that don't use them.
- */
-#definemajor(x)((int)(((u_int)(x) >> 8)&0xff)) /* major number 
*/
-#defineminor(x)((int)((x)&0x00ff)) /* minor number 
*/
-#definemakedev(x,y)((dev_t)(((x) << 8) | (y))) /* create dev_t 
*/
+#definemajor(x)((int)((dev_t)(x) >> 32))   /* major number 
*/
+#defineminor(x)((int)((x) & 0x))   /* minor number 
*/
+#definemakedev(x, y)   (((dev_t)(x) << 32) | (y))  /* create dev_t 
*/
 
 /*
  * These declarations belong elsewhere, but are repeated here and in
___
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"