diff mbox series

nilfs2: do not use unexported cpu_to_le32()/le32_to_cpu() in uapi header

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

Commit Message

Masahiro Yamada June 5, 2019, 5:30 a.m. UTC
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

Comments

Masahiro Yamada July 7, 2019, 2:40 a.m. UTC | #1
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
Andrew Morton July 7, 2019, 3:37 a.m. UTC | #2
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?
Masahiro Yamada July 7, 2019, 4:30 a.m. UTC | #3
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
Ryusuke Konishi July 7, 2019, 6:11 a.m. UTC | #4
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
Masahiro Yamada July 7, 2019, 6:41 a.m. UTC | #5
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
Ryusuke Konishi July 7, 2019, 8:48 a.m. UTC | #6
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 mbox series

Patch

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);
 }
 
 /**