ioctl.h: add missing kernel compatibility header for BUILD_ASSERT

Commit Message

Sergei Trofimovich Oct. 24, 2016, 8:29 a.m. UTC

From: Sergei Trofimovich <[e-mail]>

Header breakage noticed by cynede. Reproducible as:

    $ gcc -c /usr/include/btrfs/ioctl.h -o /tmp/a.o
        /usr/include/btrfs/ioctl.h:42:14: error: expected declaration specifiers or '...' before 'sizeof'
     BUILD_ASSERT(sizeof(struct btrfs_ioctl_vol_args) == 4096);
                  ^~~~~~

Basically gcc tries to say us BUILD_ASSERT is not visible.

BUILD_ASSERT lives in kerncompat.h which this change adds.

Reported-by: Mikhail Pukhlikov <[e-mail]>
Signed-off-by: Sergei Trofimovich <[e-mail]>
---
 ioctl.h | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

David Sterba Oct. 24, 2016, 12:52 p.m. UTC | #1

Hi,

On Mon, Oct 24, 2016 at 09:29:12AM +0100, [e-mail] wrote:
> From: Sergei Trofimovich <[e-mail]>
> 
> Header breakage noticed by cynede. Reproducible as:
> 
>     $ gcc -c /usr/include/btrfs/ioctl.h -o /tmp/a.o
>         /usr/include/btrfs/ioctl.h:42:14: error: expected declaration specifiers or '...' before 'sizeof'
>      BUILD_ASSERT(sizeof(struct btrfs_ioctl_vol_args) == 4096);
>                   ^~~~~~
> 
> Basically gcc tries to say us BUILD_ASSERT is not visible.
> 
> BUILD_ASSERT lives in kerncompat.h which this change adds.

I think including the kerncompat.h is too intrusive here, I've fixed by
providing an empty macro if it's not defined. I'll release 4.8.2 soon.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Sergei Trofimovich Nov. 12, 2016, 10:14 p.m. UTC | #2

On Mon, 24 Oct 2016 14:52:05 +0200
David Sterba <[e-mail]> wrote:

> Hi,
> 
> On Mon, Oct 24, 2016 at 09:29:12AM +0100, [e-mail] wrote:
> > From: Sergei Trofimovich <[e-mail]>
> > 
> > Header breakage noticed by cynede. Reproducible as:
> > 
> >     $ gcc -c /usr/include/btrfs/ioctl.h -o /tmp/a.o
> >         /usr/include/btrfs/ioctl.h:42:14: error: expected declaration specifiers or '...' before 'sizeof'
> >      BUILD_ASSERT(sizeof(struct btrfs_ioctl_vol_args) == 4096);
> >                   ^~~~~~
> > 
> > Basically gcc tries to say us BUILD_ASSERT is not visible.
> > 
> > BUILD_ASSERT lives in kerncompat.h which this change adds.  
> 
> I think including the kerncompat.h is too intrusive here, I've fixed by
> providing an empty macro if it's not defined. I'll release 4.8.2 soon.

Apologies. I did not test your fix right afterwards. Seems now header is incomplete
due to missing NULL (gcc-6):

btrfs-progs-v4.8.3 $ gcc -c ioctl.h -o /tmp/a.o
ioctl.h: In function 'btrfs_err_str':
ioctl.h:711:11: error: 'NULL' undeclared (first use in this function)
    return NULL;
           ^~~~
ioctl.h:711:11: note: each undeclared identifier is reported only once for each function it appears in

David Sterba Nov. 14, 2016, 12:34 p.m. UTC | #3

On Sat, Nov 12, 2016 at 10:14:49PM +0000, Sergei Trofimovich wrote:
> > > Basically gcc tries to say us BUILD_ASSERT is not visible.
> > > 
> > > BUILD_ASSERT lives in kerncompat.h which this change adds.  
> > 
> > I think including the kerncompat.h is too intrusive here, I've fixed by
> > providing an empty macro if it's not defined. I'll release 4.8.2 soon.
> 
> Apologies. I did not test your fix right afterwards. Seems now header is incomplete
> due to missing NULL (gcc-6):
> 
> btrfs-progs-v4.8.3 $ gcc -c ioctl.h -o /tmp/a.o
> ioctl.h: In function 'btrfs_err_str':
> ioctl.h:711:11: error: 'NULL' undeclared (first use in this function)
>     return NULL;
>            ^~~~
> ioctl.h:711:11: note: each undeclared identifier is reported only once for each function it appears in

The ioctl.h file can be included in both C and C++ code, I'd rahter
avoid to ifdef the right definition of NULL, so s/NULL/0/ seems as a
best fix to me.

Compiling with g++ shows other type errors like

ioctl.h:709:5: warning: ISO C++ forbids converting a string constant to ‘char*’ [-Wwrite-strings]
     "in progress";

so we'd have to change the type to 'const char*'. I'm not sure how much
breakage this could cause in programs using the header.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

David Sterba Nov. 14, 2016, 3:50 p.m. UTC | #4

On Mon, Nov 14, 2016 at 01:34:49PM +0100, David Sterba wrote:
> On Sat, Nov 12, 2016 at 10:14:49PM +0000, Sergei Trofimovich wrote:
> > > > Basically gcc tries to say us BUILD_ASSERT is not visible.
> > > > 
> > > > BUILD_ASSERT lives in kerncompat.h which this change adds.  
> > > 
> > > I think including the kerncompat.h is too intrusive here, I've fixed by
> > > providing an empty macro if it's not defined. I'll release 4.8.2 soon.
> > 
> > Apologies. I did not test your fix right afterwards. Seems now header is incomplete
> > due to missing NULL (gcc-6):
> > 
> > btrfs-progs-v4.8.3 $ gcc -c ioctl.h -o /tmp/a.o
> > ioctl.h: In function 'btrfs_err_str':
> > ioctl.h:711:11: error: 'NULL' undeclared (first use in this function)
> >     return NULL;
> >            ^~~~
> > ioctl.h:711:11: note: each undeclared identifier is reported only once for each function it appears in
> 
> The ioctl.h file can be included in both C and C++ code, I'd rahter
> avoid to ifdef the right definition of NULL, so s/NULL/0/ seems as a
> best fix to me.

Or #include <stddef.h>, now committed to devel.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/ioctl.h b/ioctl.h
index 238e7ef..ee650a9 100644
--- a/ioctl.h
+++ b/ioctl.h
@@ -26,6 +26,12 @@  extern "C" {
 #include <asm/types.h>
 #include <linux/ioctl.h>
 
+#if BTRFS_FLAT_INCLUDES
+#include "kerncompat.h"
+#else
+#include <btrfs/kerncompat.h>
+#endif /* BTRFS_FLAT_INCLUDES */
+
 #ifndef __user
 #define __user
 #endif