Message ID | 20190605053006.14332-1-yamada.masahiro@socionext.com |
---|---|
State | Accepted |
Commit | c32cc30c0544f13982ee0185d55f4910319b1a79 |
Headers | show |
Series | nilfs2: do not use unexported cpu_to_le32()/le32_to_cpu() in uapi header | expand |
Hi Andrew, Could you pick up this? The maintainer of nilfs2 is silent, but this is obviously build error. Masahiro Yamada On Wed, Jun 5, 2019 at 2:32 PM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > cpu_to_le32/le32_to_cpu is defined in include/linux/byteorder/generic.h, > which is not exported to user-space. > > UAPI headers must use the ones prefixed with double-underscore. > > Detected by compile-testing exported headers: > > ./usr/include/linux/nilfs2_ondisk.h: In function ‘nilfs_checkpoint_set_snapshot’: > ./usr/include/linux/nilfs2_ondisk.h:536:17: error: implicit declaration of function ‘cpu_to_le32’ [-Werror=implicit-function-declaration] > cp->cp_flags = cpu_to_le32(le32_to_cpu(cp->cp_flags) | \ > ^ > ./usr/include/linux/nilfs2_ondisk.h:552:1: note: in expansion of macro ‘NILFS_CHECKPOINT_FNS’ > NILFS_CHECKPOINT_FNS(SNAPSHOT, snapshot) > ^~~~~~~~~~~~~~~~~~~~ > ./usr/include/linux/nilfs2_ondisk.h:536:29: error: implicit declaration of function ‘le32_to_cpu’ [-Werror=implicit-function-declaration] > cp->cp_flags = cpu_to_le32(le32_to_cpu(cp->cp_flags) | \ > ^ > ./usr/include/linux/nilfs2_ondisk.h:552:1: note: in expansion of macro ‘NILFS_CHECKPOINT_FNS’ > NILFS_CHECKPOINT_FNS(SNAPSHOT, snapshot) > ^~~~~~~~~~~~~~~~~~~~ > ./usr/include/linux/nilfs2_ondisk.h: In function ‘nilfs_segment_usage_set_clean’: > ./usr/include/linux/nilfs2_ondisk.h:622:19: error: implicit declaration of function ‘cpu_to_le64’ [-Werror=implicit-function-declaration] > su->su_lastmod = cpu_to_le64(0); > ^~~~~~~~~~~ > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > --- > > include/uapi/linux/nilfs2_ondisk.h | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) > > diff --git a/include/uapi/linux/nilfs2_ondisk.h b/include/uapi/linux/nilfs2_ondisk.h > index a7e66ab11d1d..c23f91ae5fe8 100644 > --- a/include/uapi/linux/nilfs2_ondisk.h > +++ b/include/uapi/linux/nilfs2_ondisk.h > @@ -29,7 +29,7 @@ > > #include <linux/types.h> > #include <linux/magic.h> > - > +#include <asm/byteorder.h> > > #define NILFS_INODE_BMAP_SIZE 7 > > @@ -533,19 +533,19 @@ enum { > static inline void \ > nilfs_checkpoint_set_##name(struct nilfs_checkpoint *cp) \ > { \ > - cp->cp_flags = cpu_to_le32(le32_to_cpu(cp->cp_flags) | \ > - (1UL << NILFS_CHECKPOINT_##flag)); \ > + cp->cp_flags = __cpu_to_le32(__le32_to_cpu(cp->cp_flags) | \ > + (1UL << NILFS_CHECKPOINT_##flag)); \ > } \ > static inline void \ > nilfs_checkpoint_clear_##name(struct nilfs_checkpoint *cp) \ > { \ > - cp->cp_flags = cpu_to_le32(le32_to_cpu(cp->cp_flags) & \ > + cp->cp_flags = __cpu_to_le32(__le32_to_cpu(cp->cp_flags) & \ > ~(1UL << NILFS_CHECKPOINT_##flag)); \ > } \ > static inline int \ > nilfs_checkpoint_##name(const struct nilfs_checkpoint *cp) \ > { \ > - return !!(le32_to_cpu(cp->cp_flags) & \ > + return !!(__le32_to_cpu(cp->cp_flags) & \ > (1UL << NILFS_CHECKPOINT_##flag)); \ > } > > @@ -595,20 +595,20 @@ enum { > static inline void \ > nilfs_segment_usage_set_##name(struct nilfs_segment_usage *su) \ > { \ > - su->su_flags = cpu_to_le32(le32_to_cpu(su->su_flags) | \ > + su->su_flags = __cpu_to_le32(__le32_to_cpu(su->su_flags) | \ > (1UL << NILFS_SEGMENT_USAGE_##flag));\ > } \ > static inline void \ > nilfs_segment_usage_clear_##name(struct nilfs_segment_usage *su) \ > { \ > su->su_flags = \ > - cpu_to_le32(le32_to_cpu(su->su_flags) & \ > + __cpu_to_le32(__le32_to_cpu(su->su_flags) & \ > ~(1UL << NILFS_SEGMENT_USAGE_##flag)); \ > } \ > static inline int \ > nilfs_segment_usage_##name(const struct nilfs_segment_usage *su) \ > { \ > - return !!(le32_to_cpu(su->su_flags) & \ > + return !!(__le32_to_cpu(su->su_flags) & \ > (1UL << NILFS_SEGMENT_USAGE_##flag)); \ > } > > @@ -619,15 +619,15 @@ NILFS_SEGMENT_USAGE_FNS(ERROR, error) > static inline void > nilfs_segment_usage_set_clean(struct nilfs_segment_usage *su) > { > - su->su_lastmod = cpu_to_le64(0); > - su->su_nblocks = cpu_to_le32(0); > - su->su_flags = cpu_to_le32(0); > + su->su_lastmod = __cpu_to_le64(0); > + su->su_nblocks = __cpu_to_le32(0); > + su->su_flags = __cpu_to_le32(0); > } > > static inline int > nilfs_segment_usage_clean(const struct nilfs_segment_usage *su) > { > - return !le32_to_cpu(su->su_flags); > + return !__le32_to_cpu(su->su_flags); > } > > /** > -- > 2.17.1 > -- Best Regards Masahiro Yamada
On Wed, 5 Jun 2019 14:30:06 +0900 Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > cpu_to_le32/le32_to_cpu is defined in include/linux/byteorder/generic.h, > which is not exported to user-space. > > UAPI headers must use the ones prefixed with double-underscore. > > Detected by compile-testing exported headers: > > ./usr/include/linux/nilfs2_ondisk.h: In function ‘nilfs_checkpoint_set_snapshot’: > ./usr/include/linux/nilfs2_ondisk.h:536:17: error: implicit declaration of function ‘cpu_to_le32’ [-Werror=implicit-function-declaration] > cp->cp_flags = cpu_to_le32(le32_to_cpu(cp->cp_flags) | \ > ^ > ./usr/include/linux/nilfs2_ondisk.h:552:1: note: in expansion of macro ‘NILFS_CHECKPOINT_FNS’ > NILFS_CHECKPOINT_FNS(SNAPSHOT, snapshot) > ^~~~~~~~~~~~~~~~~~~~ > ./usr/include/linux/nilfs2_ondisk.h:536:29: error: implicit declaration of function ‘le32_to_cpu’ [-Werror=implicit-function-declaration] > cp->cp_flags = cpu_to_le32(le32_to_cpu(cp->cp_flags) | \ > ^ > ./usr/include/linux/nilfs2_ondisk.h:552:1: note: in expansion of macro ‘NILFS_CHECKPOINT_FNS’ > NILFS_CHECKPOINT_FNS(SNAPSHOT, snapshot) > ^~~~~~~~~~~~~~~~~~~~ > ./usr/include/linux/nilfs2_ondisk.h: In function ‘nilfs_segment_usage_set_clean’: > ./usr/include/linux/nilfs2_ondisk.h:622:19: error: implicit declaration of function ‘cpu_to_le64’ [-Werror=implicit-function-declaration] > su->su_lastmod = cpu_to_le64(0); > ^~~~~~~~~~~ Seems fairly serious. I'm thinking this needs a cc:stable?
Hi Andrew, On Sun, Jul 7, 2019 at 12:38 PM Andrew Morton <akpm@linux-foundation.org> wrote: > > Seems fairly serious. I'm thinking this needs a cc:stable? Ah, yes. Cc: linux-stable <stable@vger.kernel.org> # 4.9 Fixes: e63e88bc53ba ("nilfs2: move ioctl interface and disk layout to uapi separately") Thanks. -- Best Regards Masahiro Yamada
Yamada-san, I'm sorry for not being able to follow. Could you tell me how did you find the build error? Still I cannot reproduce the error in my environment. Or, if it's right that we should not use cpu_to_le{16,32,64}/le{16,32,64}_to_cpu() in UAPI header files, I will acknowledge the change and modify nilfs utilities to comply with the guideline. Thanks, Ryusuke Konishi 2019年7月7日(日) 13:31 Masahiro Yamada <yamada.masahiro@socionext.com>: > > Hi Andrew, > > On Sun, Jul 7, 2019 at 12:38 PM Andrew Morton <akpm@linux-foundation.org> wrote: > > > > > Seems fairly serious. I'm thinking this needs a cc:stable? > > > Ah, yes. > > > Cc: linux-stable <stable@vger.kernel.org> # 4.9 > Fixes: e63e88bc53ba ("nilfs2: move ioctl interface and disk layout to > uapi separately") > > > Thanks. > > -- > Best Regards > Masahiro Yamada
Konishi-san, On Sun, Jul 7, 2019 at 3:11 PM Ryusuke Konishi <konishi.ryusuke@lab.ntt.co.jp> wrote: > > Yamada-san, > > I'm sorry for not being able to follow. > > Could you tell me how did you find the build error? > Still I cannot reproduce the error in my environment. "You can compile UAPI headers in kernel-space" does not necessarily mean "you can compile them in user-space". To make sure UAPI headers can be included from libc etc. you must compile them after 'make headers_install', but people often miss to test that. To automate this testing, I will merge the following for 5.3-rc1: https://patchwork.kernel.org/patch/11024715/ Once it lands in upstream, all the new breakage will be caught by 0-day bot. Currently, nilfs2_ondisk.h is excluded from the test coverage since it is broken. Thanks. Masahiro Yamada > Or, if it's right that we should not use > cpu_to_le{16,32,64}/le{16,32,64}_to_cpu() in UAPI header files, > I will acknowledge the change and modify nilfs utilities to comply > with the guideline. > > Thanks, > Ryusuke Konishi > > 2019年7月7日(日) 13:31 Masahiro Yamada <yamada.masahiro@socionext.com>: > > > > Hi Andrew, > > > > On Sun, Jul 7, 2019 at 12:38 PM Andrew Morton <akpm@linux-foundation.org> wrote: > > > > > > > > Seems fairly serious. I'm thinking this needs a cc:stable? > > > > > > Ah, yes. > > > > > > Cc: linux-stable <stable@vger.kernel.org> # 4.9 > > Fixes: e63e88bc53ba ("nilfs2: move ioctl interface and disk layout to > > uapi separately") > > > > > > Thanks. > > > > -- > > Best Regards > > Masahiro Yamada -- Best Regards Masahiro Yamada
Yamada-san, > To automate this testing, I will merge the following for 5.3-rc1: > https://patchwork.kernel.org/patch/11024715/ Thank you. I could get the series through linux-next tree, and could reproduce the build error by enabling the new coverage test for nilfs2_ondisk.h. Acked-by: Ryusuke Konishi <konishi.ryusuke@gmail.com> Regards, Ryusuke Konishi 2019年7月7日(日) 15:42 Masahiro Yamada <yamada.masahiro@socionext.com>: > > Konishi-san, > > > On Sun, Jul 7, 2019 at 3:11 PM Ryusuke Konishi > <konishi.ryusuke@lab.ntt.co.jp> wrote: > > > > Yamada-san, > > > > I'm sorry for not being able to follow. > > > > Could you tell me how did you find the build error? > > Still I cannot reproduce the error in my environment. > > > "You can compile UAPI headers in kernel-space" does not necessarily mean > "you can compile them in user-space". > > To make sure UAPI headers can be included from libc etc. > you must compile them after 'make headers_install', > but people often miss to test that. > > To automate this testing, I will merge the following for 5.3-rc1: > https://patchwork.kernel.org/patch/11024715/ > > Once it lands in upstream, all the new breakage > will be caught by 0-day bot. > > Currently, nilfs2_ondisk.h is excluded from the test coverage > since it is broken. > > Thanks. > Masahiro Yamada > > > > > Or, if it's right that we should not use > > cpu_to_le{16,32,64}/le{16,32,64}_to_cpu() in UAPI header files, > > I will acknowledge the change and modify nilfs utilities to comply > > with the guideline. > > > > Thanks, > > Ryusuke Konishi > > > > 2019年7月7日(日) 13:31 Masahiro Yamada <yamada.masahiro@socionext.com>: > > > > > > Hi Andrew, > > > > > > On Sun, Jul 7, 2019 at 12:38 PM Andrew Morton <akpm@linux-foundation.org> wrote: > > > > > > > > > > > Seems fairly serious. I'm thinking this needs a cc:stable? > > > > > > > > > Ah, yes. > > > > > > > > > Cc: linux-stable <stable@vger.kernel.org> # 4.9 > > > Fixes: e63e88bc53ba ("nilfs2: move ioctl interface and disk layout to > > > uapi separately") > > > > > > > > > Thanks. > > > > > > -- > > > Best Regards > > > Masahiro Yamada > > > > -- > Best Regards > Masahiro Yamada
diff --git a/include/uapi/linux/nilfs2_ondisk.h b/include/uapi/linux/nilfs2_ondisk.h index a7e66ab11d1d..c23f91ae5fe8 100644 --- a/include/uapi/linux/nilfs2_ondisk.h +++ b/include/uapi/linux/nilfs2_ondisk.h @@ -29,7 +29,7 @@ #include <linux/types.h> #include <linux/magic.h> - +#include <asm/byteorder.h> #define NILFS_INODE_BMAP_SIZE 7 @@ -533,19 +533,19 @@ enum { static inline void \ nilfs_checkpoint_set_##name(struct nilfs_checkpoint *cp) \ { \ - cp->cp_flags = cpu_to_le32(le32_to_cpu(cp->cp_flags) | \ - (1UL << NILFS_CHECKPOINT_##flag)); \ + cp->cp_flags = __cpu_to_le32(__le32_to_cpu(cp->cp_flags) | \ + (1UL << NILFS_CHECKPOINT_##flag)); \ } \ static inline void \ nilfs_checkpoint_clear_##name(struct nilfs_checkpoint *cp) \ { \ - cp->cp_flags = cpu_to_le32(le32_to_cpu(cp->cp_flags) & \ + cp->cp_flags = __cpu_to_le32(__le32_to_cpu(cp->cp_flags) & \ ~(1UL << NILFS_CHECKPOINT_##flag)); \ } \ static inline int \ nilfs_checkpoint_##name(const struct nilfs_checkpoint *cp) \ { \ - return !!(le32_to_cpu(cp->cp_flags) & \ + return !!(__le32_to_cpu(cp->cp_flags) & \ (1UL << NILFS_CHECKPOINT_##flag)); \ } @@ -595,20 +595,20 @@ enum { static inline void \ nilfs_segment_usage_set_##name(struct nilfs_segment_usage *su) \ { \ - su->su_flags = cpu_to_le32(le32_to_cpu(su->su_flags) | \ + su->su_flags = __cpu_to_le32(__le32_to_cpu(su->su_flags) | \ (1UL << NILFS_SEGMENT_USAGE_##flag));\ } \ static inline void \ nilfs_segment_usage_clear_##name(struct nilfs_segment_usage *su) \ { \ su->su_flags = \ - cpu_to_le32(le32_to_cpu(su->su_flags) & \ + __cpu_to_le32(__le32_to_cpu(su->su_flags) & \ ~(1UL << NILFS_SEGMENT_USAGE_##flag)); \ } \ static inline int \ nilfs_segment_usage_##name(const struct nilfs_segment_usage *su) \ { \ - return !!(le32_to_cpu(su->su_flags) & \ + return !!(__le32_to_cpu(su->su_flags) & \ (1UL << NILFS_SEGMENT_USAGE_##flag)); \ } @@ -619,15 +619,15 @@ NILFS_SEGMENT_USAGE_FNS(ERROR, error) static inline void nilfs_segment_usage_set_clean(struct nilfs_segment_usage *su) { - su->su_lastmod = cpu_to_le64(0); - su->su_nblocks = cpu_to_le32(0); - su->su_flags = cpu_to_le32(0); + su->su_lastmod = __cpu_to_le64(0); + su->su_nblocks = __cpu_to_le32(0); + su->su_flags = __cpu_to_le32(0); } static inline int nilfs_segment_usage_clean(const struct nilfs_segment_usage *su) { - return !le32_to_cpu(su->su_flags); + return !__le32_to_cpu(su->su_flags); } /**
cpu_to_le32/le32_to_cpu is defined in include/linux/byteorder/generic.h, which is not exported to user-space. UAPI headers must use the ones prefixed with double-underscore. Detected by compile-testing exported headers: ./usr/include/linux/nilfs2_ondisk.h: In function ‘nilfs_checkpoint_set_snapshot’: ./usr/include/linux/nilfs2_ondisk.h:536:17: error: implicit declaration of function ‘cpu_to_le32’ [-Werror=implicit-function-declaration] cp->cp_flags = cpu_to_le32(le32_to_cpu(cp->cp_flags) | \ ^ ./usr/include/linux/nilfs2_ondisk.h:552:1: note: in expansion of macro ‘NILFS_CHECKPOINT_FNS’ NILFS_CHECKPOINT_FNS(SNAPSHOT, snapshot) ^~~~~~~~~~~~~~~~~~~~ ./usr/include/linux/nilfs2_ondisk.h:536:29: error: implicit declaration of function ‘le32_to_cpu’ [-Werror=implicit-function-declaration] cp->cp_flags = cpu_to_le32(le32_to_cpu(cp->cp_flags) | \ ^ ./usr/include/linux/nilfs2_ondisk.h:552:1: note: in expansion of macro ‘NILFS_CHECKPOINT_FNS’ NILFS_CHECKPOINT_FNS(SNAPSHOT, snapshot) ^~~~~~~~~~~~~~~~~~~~ ./usr/include/linux/nilfs2_ondisk.h: In function ‘nilfs_segment_usage_set_clean’: ./usr/include/linux/nilfs2_ondisk.h:622:19: error: implicit declaration of function ‘cpu_to_le64’ [-Werror=implicit-function-declaration] su->su_lastmod = cpu_to_le64(0); ^~~~~~~~~~~ Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> --- include/uapi/linux/nilfs2_ondisk.h | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) -- 2.17.1