[patch 27/28] Introduce U16_MAX and U32_MAX

[patch 27/28] Introduce U16_MAX and U32_MAX

[akpm]

From: Satyam Sharma <satyam@infradead.org>

... in kernel.h and clean up home-grown macros elsewhere in the tree.

Leave out the one in reiserfs_fs.h as it is in the userspace-visible part
of that header. Still, #undef the (equivalent) kernel version there to
avoid seeing "redefined, previous definition was here" gcc warnings.

[akpm@linux-foundation.org: fix U16_MAX, U32_MAX defns]
Signed-off-by: Satyam Sharma <satyam@infradead.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/net/netconsole.c    |    7 +++----
 include/linux/kernel.h      |    3 +++
 include/linux/reiserfs_fs.h |    1 +
 net/ipv4/tcp_illinois.c     |    2 +-
 4 files changed, 8 insertions(+), 5 deletions(-)

diff -puN drivers/net/netconsole.c~introduce-u16_max-and-u32_max drivers/net/netconsole.c
--- a/drivers/net/netconsole.c~introduce-u16_max-and-u32_max
+++ a/drivers/net/netconsole.c
@@ -34,6 +34,7 @@
  *
  ****************************************************************/
 
+#include <linux/kernel.h>
 #include <linux/mm.h>
 #include <linux/init.h>
 #include <linux/module.h>
@@ -389,7 +390,6 @@ static ssize_t store_local_port(struct n
 				size_t count)
 {
 	long local_port;
-#define __U16_MAX	((__u16) ~0U)
 
 	if (nt->enabled) {
 		printk(KERN_ERR "netconsole: target (%s) is enabled, "
@@ -398,7 +398,7 @@ static ssize_t store_local_port(struct n
 		return -EINVAL;
 	}
 
-	local_port = strtol10_check_range(buf, 0, __U16_MAX);
+	local_port = strtol10_check_range(buf, 0, U16_MAX);
 	if (local_port < 0)
 		return local_port;
 
@@ -412,7 +412,6 @@ static ssize_t store_remote_port(struct 
 				 size_t count)
 {
 	long remote_port;
-#define __U16_MAX	((__u16) ~0U)
 
 	if (nt->enabled) {
 		printk(KERN_ERR "netconsole: target (%s) is enabled, "
@@ -421,7 +420,7 @@ static ssize_t store_remote_port(struct 
 		return -EINVAL;
 	}
 
-	remote_port = strtol10_check_range(buf, 0, __U16_MAX);
+	remote_port = strtol10_check_range(buf, 0, U16_MAX);
 	if (remote_port < 0)
 		return remote_port;
 
diff -puN include/linux/kernel.h~introduce-u16_max-and-u32_max include/linux/kernel.h
--- a/include/linux/kernel.h~introduce-u16_max-and-u32_max
+++ a/include/linux/kernel.h
@@ -30,6 +30,9 @@ extern const char linux_proc_banner[];
 #define LLONG_MIN	(-LLONG_MAX - 1)
 #define ULLONG_MAX	(~0ULL)
 
+#define U16_MAX		(0xffff)
+#define U32_MAX		(0xffffffff)
+
 #define STACK_MAGIC	0xdeadbeef
 
 #define ALIGN(x,a)		__ALIGN_MASK(x,(typeof(x))(a)-1)
diff -puN include/linux/reiserfs_fs.h~introduce-u16_max-and-u32_max include/linux/reiserfs_fs.h
--- a/include/linux/reiserfs_fs.h~introduce-u16_max-and-u32_max
+++ a/include/linux/reiserfs_fs.h
@@ -1225,6 +1225,7 @@ struct treepath var = {.path_length = IL
 #define MAX_US_INT 0xffff
 
 // reiserfs version 2 has max offset 60 bits. Version 1 - 32 bit offset
+#undef	U32_MAX
 #define U32_MAX (~(__u32)0)
 
 static inline loff_t max_reiserfs_offset(struct inode *inode)
diff -puN net/ipv4/tcp_illinois.c~introduce-u16_max-and-u32_max net/ipv4/tcp_illinois.c
--- a/net/ipv4/tcp_illinois.c~introduce-u16_max-and-u32_max
+++ a/net/ipv4/tcp_illinois.c
@@ -12,6 +12,7 @@
  * Copyright (C) 2007 Stephen Hemminger <shemminger@linux-foundation.org>
  */
 
+#include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/skbuff.h>
 #include <linux/inet_diag.h>
@@ -23,7 +24,6 @@
 #define ALPHA_MIN	((3*ALPHA_SCALE)/10)	/* ~0.3 */
 #define ALPHA_MAX	(10*ALPHA_SCALE)	/* 10.0 */
 #define ALPHA_BASE	ALPHA_SCALE		/* 1.0 */
-#define U32_MAX		((u32)~0U)
 #define RTT_MAX		(U32_MAX / ALPHA_MAX)	/* 3.3 secs */
 
 #define BETA_SHIFT	6
_

Re: [patch 27/28] Introduce U16_MAX and U32_MAX

[David Miller]

From: akpm@linux-foundation.org
Date: Fri, 10 Aug 2007 14:12:10 -0700

> From: Satyam Sharma <satyam@infradead.org>
> 
> ... in kernel.h and clean up home-grown macros elsewhere in the tree.
> 
> Leave out the one in reiserfs_fs.h as it is in the userspace-visible part
> of that header. Still, #undef the (equivalent) kernel version there to
> avoid seeing "redefined, previous definition was here" gcc warnings.
> 
> [akpm@linux-foundation.org: fix U16_MAX, U32_MAX defns]
> Signed-off-by: Satyam Sharma <satyam@infradead.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

I won't apply this one, for two reasons:

1) The reiserfs definition is better, it is _type_ based.
   Please use (~(__u16)0) and (~(__u32)0), respectively.

2) The reiserfs definition is going to define an equivalent
   value, so just adding an #undef and still letting reiserfs
   override is wrong.  Why put a common define in kernel.h
   if other headers still keep their own crufty copy too?

Re: [patch 27/28] Introduce U16_MAX and U32_MAX

[Andrew Morton]

On Fri, 10 Aug 2007 15:38:19 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:

> From: akpm@linux-foundation.org
> Date: Fri, 10 Aug 2007 14:12:10 -0700
> 
> > From: Satyam Sharma <satyam@infradead.org>
> > 
> > ... in kernel.h and clean up home-grown macros elsewhere in the tree.
> > 
> > Leave out the one in reiserfs_fs.h as it is in the userspace-visible part
> > of that header. Still, #undef the (equivalent) kernel version there to
> > avoid seeing "redefined, previous definition was here" gcc warnings.
> > 
> > [akpm@linux-foundation.org: fix U16_MAX, U32_MAX defns]
> > Signed-off-by: Satyam Sharma <satyam@infradead.org>
> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> 
> I won't apply this one, for two reasons:
> 
> 1) The reiserfs definition is better, it is _type_ based.
>    Please use (~(__u16)0) and (~(__u32)0), respectively.
> 
> 2) The reiserfs definition is going to define an equivalent
>    value, so just adding an #undef and still letting reiserfs
>    override is wrong.  Why put a common define in kernel.h
>    if other headers still keep their own crufty copy too?

OK, I made those changes.  Will add to the next batch->davem.

I note that reiserfs has open-coded (~(__u64) 0) in there too...



From: Satyam Sharma <satyam@infradead.org>

... in kernel.h and clean up home-grown macros elsewhere in the tree.

Signed-off-by: Satyam Sharma <satyam@infradead.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/net/netconsole.c    |    7 +++----
 include/linux/kernel.h      |    3 +++
 include/linux/reiserfs_fs.h |    2 --
 net/ipv4/tcp_illinois.c     |    2 +-
 4 files changed, 7 insertions(+), 7 deletions(-)

diff -puN drivers/net/netconsole.c~introduce-u16_max-and-u32_max drivers/net/netconsole.c
--- a/drivers/net/netconsole.c~introduce-u16_max-and-u32_max
+++ a/drivers/net/netconsole.c
@@ -34,6 +34,7 @@
  *
  ****************************************************************/
 
+#include <linux/kernel.h>
 #include <linux/mm.h>
 #include <linux/init.h>
 #include <linux/module.h>
@@ -389,7 +390,6 @@ static ssize_t store_local_port(struct n
 				size_t count)
 {
 	long local_port;
-#define __U16_MAX	((__u16) ~0U)
 
 	if (nt->enabled) {
 		printk(KERN_ERR "netconsole: target (%s) is enabled, "
@@ -398,7 +398,7 @@ static ssize_t store_local_port(struct n
 		return -EINVAL;
 	}
 
-	local_port = strtol10_check_range(buf, 0, __U16_MAX);
+	local_port = strtol10_check_range(buf, 0, U16_MAX);
 	if (local_port < 0)
 		return local_port;
 
@@ -412,7 +412,6 @@ static ssize_t store_remote_port(struct 
 				 size_t count)
 {
 	long remote_port;
-#define __U16_MAX	((__u16) ~0U)
 
 	if (nt->enabled) {
 		printk(KERN_ERR "netconsole: target (%s) is enabled, "
@@ -421,7 +420,7 @@ static ssize_t store_remote_port(struct 
 		return -EINVAL;
 	}
 
-	remote_port = strtol10_check_range(buf, 0, __U16_MAX);
+	remote_port = strtol10_check_range(buf, 0, U16_MAX);
 	if (remote_port < 0)
 		return remote_port;
 
diff -puN include/linux/kernel.h~introduce-u16_max-and-u32_max include/linux/kernel.h
--- a/include/linux/kernel.h~introduce-u16_max-and-u32_max
+++ a/include/linux/kernel.h
@@ -30,6 +30,9 @@ extern const char linux_proc_banner[];
 #define LLONG_MIN	(-LLONG_MAX - 1)
 #define ULLONG_MAX	(~0ULL)
 
+#define U16_MAX		(~(__u16)0)
+#define U32_MAX		(~(__u32)0)
+
 #define STACK_MAGIC	0xdeadbeef
 
 #define ALIGN(x,a)		__ALIGN_MASK(x,(typeof(x))(a)-1)
diff -puN include/linux/reiserfs_fs.h~introduce-u16_max-and-u32_max include/linux/reiserfs_fs.h
--- a/include/linux/reiserfs_fs.h~introduce-u16_max-and-u32_max
+++ a/include/linux/reiserfs_fs.h
@@ -1225,8 +1225,6 @@ struct treepath var = {.path_length = IL
 #define MAX_US_INT 0xffff
 
 // reiserfs version 2 has max offset 60 bits. Version 1 - 32 bit offset
-#define U32_MAX (~(__u32)0)
-
 static inline loff_t max_reiserfs_offset(struct inode *inode)
 {
 	if (get_inode_item_key_version(inode) == KEY_FORMAT_3_5)
diff -puN net/ipv4/tcp_illinois.c~introduce-u16_max-and-u32_max net/ipv4/tcp_illinois.c
--- a/net/ipv4/tcp_illinois.c~introduce-u16_max-and-u32_max
+++ a/net/ipv4/tcp_illinois.c
@@ -12,6 +12,7 @@
  * Copyright (C) 2007 Stephen Hemminger <shemminger@linux-foundation.org>
  */
 
+#include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/skbuff.h>
 #include <linux/inet_diag.h>
@@ -23,7 +24,6 @@
 #define ALPHA_MIN	((3*ALPHA_SCALE)/10)	/* ~0.3 */
 #define ALPHA_MAX	(10*ALPHA_SCALE)	/* 10.0 */
 #define ALPHA_BASE	ALPHA_SCALE		/* 1.0 */
-#define U32_MAX		((u32)~0U)
 #define RTT_MAX		(U32_MAX / ALPHA_MAX)	/* 3.3 secs */
 
 #define BETA_SHIFT	6
_

Re: [patch 27/28] Introduce U16_MAX and U32_MAX

[Satyam Sharma]

Hi David,


On Fri, 10 Aug 2007, David Miller wrote:

> From: akpm@linux-foundation.org
> Date: Fri, 10 Aug 2007 14:12:10 -0700
> 
> > From: Satyam Sharma <satyam@infradead.org>
> > 
> > ... in kernel.h and clean up home-grown macros elsewhere in the tree.
> > 
> > Leave out the one in reiserfs_fs.h as it is in the userspace-visible part
> > of that header. Still, #undef the (equivalent) kernel version there to
> > avoid seeing "redefined, previous definition was here" gcc warnings.
> > 
> > [akpm@linux-foundation.org: fix U16_MAX, U32_MAX defns]
> > Signed-off-by: Satyam Sharma <satyam@infradead.org>
> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> 
> I won't apply this one, for two reasons:
> 
> 1) The reiserfs definition is better, it is _type_ based.
>    Please use (~(__u16)0) and (~(__u32)0), respectively.

Hmm, in that case ((__u16)0xffff) and ((__u32)0xffffffff) are probably
better and clearer -- as that's what u16_max and u32_max are, after all.

We do require the (~0) thing for the max int/uint/long types, but that's
because those are types where the number-of-bits is not known when writing
the macro definition -- but that's not case with u16 and u32, so the
0xff... variants are clearer, IMHO.


> 2) The reiserfs definition is going to define an equivalent
>    value, so just adding an #undef and still letting reiserfs
>    override is wrong.  Why put a common define in kernel.h
>    if other headers still keep their own crufty copy too?

Because removing the (re-)definition of U32_MAX from in there in
reiserfs_fs.h will break builds of all userspace users of U32_MAX and
max_reiserfs_offset(), would it not? I haven't looked at any reiserfs
userspace tools source code, so possibly none such (that use
max_reiserfs_offset) exist, but I thought it better to be safe.
I'll have a look at the reiserfs-utils package, just in case.


Thanks,
Satyam

Re: [patch 27/28] Introduce U16_MAX and U32_MAX

[Satyam Sharma]

On Mon, 13 Aug 2007, Satyam Sharma wrote:

> Hi David,
> 
> 
> On Fri, 10 Aug 2007, David Miller wrote:
> [...]
> > 1) The reiserfs definition is better, it is _type_ based.
> >    Please use (~(__u16)0) and (~(__u32)0), respectively.
> 
> Hmm, in that case ((__u16)0xffff) and ((__u32)0xffffffff) are probably
> better and clearer -- as that's what u16_max and u32_max are, after all.
> 
> We do require the (~0) thing for the max int/uint/long types, but that's
> because those are types where the number-of-bits is not known when writing
> the macro definition -- but that's not case with u16 and u32, so the
> 0xff... variants are clearer, IMHO.
> 
> 
> > 2) The reiserfs definition is going to define an equivalent
> >    value, so just adding an #undef and still letting reiserfs
> >    override is wrong.  Why put a common define in kernel.h
> >    if other headers still keep their own crufty copy too?
> 
> Because removing the (re-)definition of U32_MAX from in there in
> reiserfs_fs.h will break builds of all userspace users of U32_MAX and
> max_reiserfs_offset(), would it not? I haven't looked at any reiserfs
> userspace tools source code, so possibly none such (that use
> max_reiserfs_offset) exist, but I thought it better to be safe.
> I'll have a look at the reiserfs-utils package, just in case.

I checked with latest reiserfsprogs available from namesys.com and also
reiserfs-utils package available from Fedora, and it appears there are
no users of either U32_MAX or max_reiserfs_offset() in any userspace
reiserfs tools, so we're safe in removing the U32_MAX from
include/linux/reiserfs_fs.h.


Thanks,
Satyam

Re: [patch 27/28] Introduce U16_MAX and U32_MAX

[David Miller]

From: Satyam Sharma <satyam@infradead.org>
Date: Mon, 13 Aug 2007 20:59:09 +0530 (IST)

> On Fri, 10 Aug 2007, David Miller wrote:
> 
> > 2) The reiserfs definition is going to define an equivalent
> >    value, so just adding an #undef and still letting reiserfs
> >    override is wrong.  Why put a common define in kernel.h
> >    if other headers still keep their own crufty copy too?
> 
> Because removing the (re-)definition of U32_MAX from in there in
> reiserfs_fs.h will break builds of all userspace users of U32_MAX and
> max_reiserfs_offset(), would it not? I haven't looked at any reiserfs
> userspace tools source code, so possibly none such (that use
> max_reiserfs_offset) exist, but I thought it better to be safe.
> I'll have a look at the reiserfs-utils package, just in case.

If this is the case then it would be better to pick different macro
names from the one's reiserfs already defines in it's header exported
userland interfaces.

Or, alternatively, make sure reiserfs's headers get the appropriate
headers and defines even when used in userspace.