| Message ID | 1456845641-6985-1-git-send-email-nicolas.dichtel@6wind.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, 2016-03-01 at 16:20 +0100, Nicolas Dichtel wrote:
> DIV_ROUND_UP and is defined in linux/kernel.h only for the kernel.
[...]
But what happens when another UAPI header wants to use DIV_ROUND_UP()?
Do we duplicate the definition there as well?
It seems cleaner to do something like:
--- a/include/uapi/linux/kernel.h
+++ b/include/uapi/linux/kernel.h
+#define __KERNEL_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
-#define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
+#define DIV_ROUND_UP __KERNEL_DIV_ROUND_UP
and then in include/uapi/linux/ethtool.h, replace DIV_ROUND_UP with
__KERNEL_DIV_ROUND_UP throughout.
Ben.
From: Nicolas Dichtel <nicolas.dichtel@6wind.com> Date: Tue, 1 Mar 2016 16:20:41 +0100 > DIV_ROUND_UP and is defined in linux/kernel.h only for the kernel. INT_MAX > needs limits.h in userland. It is wrong to provide a definition of this in the user visible global namespace.
On Tue, 2016-03-01 at 11:42 -0500, David Miller wrote: > From: Nicolas Dichtel <nicolas.dichtel@6wind.com> > Date: Tue, 1 Mar 2016 16:20:41 +0100 > > > DIV_ROUND_UP and is defined in linux/kernel.h only for the kernel. INT_MAX > > needs limits.h in userland. > > It is wrong to provide a definition of this in the user visible global > namespace. Which is why he's not doing that... Ben.
From: Ben Hutchings <ben@decadent.org.uk> Date: Tue, 01 Mar 2016 17:14:06 +0000 > On Tue, 2016-03-01 at 11:42 -0500, David Miller wrote: >> From: Nicolas Dichtel <nicolas.dichtel@6wind.com> >> Date: Tue, 1 Mar 2016 16:20:41 +0100 >> >> > DIV_ROUND_UP and is defined in linux/kernel.h only for the kernel. INT_MAX >> > needs limits.h in userland. >> >> It is wrong to provide a definition of this in the user visible global >> namespace. > > Which is why he's not doing that... No, that's exactly what he's doing: +#ifndef __KERNEL__ +#include <limits.h> /* for INT_MAX */ +#define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d)) +#endif in a uapi header file. "If not kernel, define DEV_ROUND_UP"
On Tue, 2016-03-01 at 14:48 -0500, David Miller wrote: > From: Ben Hutchings <ben@decadent.org.uk> > Date: Tue, 01 Mar 2016 17:14:06 +0000 > > > On Tue, 2016-03-01 at 11:42 -0500, David Miller wrote: > >> From: Nicolas Dichtel <nicolas.dichtel@6wind.com> > >> Date: Tue, 1 Mar 2016 16:20:41 +0100 > >> > >> > DIV_ROUND_UP and is defined in linux/kernel.h only for the kernel. INT_MAX > >> > needs limits.h in userland. > >> > >> It is wrong to provide a definition of this in the user visible global > >> namespace. > > > > Which is why he's not doing that... > > No, that's exactly what he's doing: > > +#ifndef __KERNEL__ > +#include <limits.h> /* for INT_MAX */ > +#define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d)) > +#endif > > in a uapi header file. > > "If not kernel, define DEV_ROUND_UP" Sorry, as there was already a v2 which doesn't do this, I thought you were responding to that. Ben.
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h index 37fd6dc33de4..d4e1351fe9b5 100644 --- a/include/uapi/linux/ethtool.h +++ b/include/uapi/linux/ethtool.h @@ -16,6 +16,11 @@ #include <linux/types.h> #include <linux/if_ether.h> +#ifndef __KERNEL__ +#include <limits.h> /* for INT_MAX */ +#define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d)) +#endif + /* All structures exposed to userland should be defined such that they * have the same layout for 32-bit and 64-bit userland. */
DIV_ROUND_UP and is defined in linux/kernel.h only for the kernel. INT_MAX needs limits.h in userland. When ethtool.h is included by a userland app, we got the following errors: include/linux/ethtool.h:1218:8: error: variably modified 'queue_mask' at file scope __u32 queue_mask[DIV_ROUND_UP(MAX_NUM_QUEUE, 32)]; ^ .../usr/include/linux/ethtool.h: In function 'ethtool_validate_speed': .../usr/include/linux/ethtool.h:1471:18: error: 'INT_MAX' undeclared (first use in this function) return speed <= INT_MAX || speed == SPEED_UNKNOWN ^ Fixes: ac2c7ad0e5d6 ("net/ethtool: introduce a new ioctl for per queue setting") Fixes: e02564ee334a ("ethtool: make validate_speed accept all speeds between 0 and INT_MAX") CC: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> CC: Kan Liang <kan.liang@intel.com> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> --- include/uapi/linux/ethtool.h | 5 +++++ 1 file changed, 5 insertions(+)