Skip to contentSkip to conversation

ioctl.h: add missing kernel compatibility header for BUILD_ASSERT

PATCH In review [email protected] 2 participants 5 messages Oct 24, 2016 – Nov 14, 2016 Btrfs file system

lore
Share & tools
lore

[email protected]

author
#0 lore raw

PATCH 1/1 ioctl.h: add missing kernel compatibility header for BUILD_ASSERT

From: Sergei Trofimovich <[email protected]>

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 <[email protected]>
Signed-off-by Sergei Trofimovich <[email protected]>
---
 ioctl.h | 6 ++++++
 1 file changed, 6 insertions(+)
ioctl.h +6 -0
index 238e7ef..ee650a9 100644
@@ -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
Signature · 1 line
2.10.1
#1 lore raw

Hi,

Quoted 4 lines · On Mon, Oct 24, 2016 at 09:29:12AM +0100, [email protected] wrote:

From: Sergei Trofimovich <[email protected]>

Header breakage noticed by cynede. Reproducible as:

Quoted 8 lines
    $ 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.

author
#2 lore raw
Quoted 12 lines · On Mon, 24 Oct 2016 14:52:05 +0200 David Sterba <[email protected]> wrote:

Hi,

Quoted 9 lines · On Mon, Oct 24, 2016 at 09:29:12AM +0100, [email protected] wrote:

From: Sergei Trofimovich <[email protected]>

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);
                  ^~~~~~
Quoted 6 lines
Quoted 3 lines

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

Signature · 1 line
  Sergei
#3 lore raw
Quoted 10 lines · Sergei Trofimovich
Quoted 6 lines
Quoted 3 lines

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):

Quoted 6 lines

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.

#4 lore raw
Quoted 18 lines · David Sterba
Quoted 10 lines · Sergei Trofimovich
Quoted 6 lines
Quoted 3 lines

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):

Quoted 6 lines

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

Quoted 3 lines

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.