Linux 3.0 commit 3627924acf70a broke include/mtd/ubi_user.h.

BUG Rob Landley 3 participants 6 messages Aug 14, 2011 – Aug 15, 2011

Share & tools

Rob Landley

author

2011-08-14 17:45 UTC #0 lore raw

"make headers_install" copies include/mtd/ubi_user.h to userspace, where things like busybox link against it. With 3.0, building defconfig busybox gets this error:

In file included from miscutils/ubi_tools.c:63:
/home/landley/aboriginal/aboriginal/build/simple-cross-compiler-i686/bin/../include/mtd/ubi-user.h:329:
error: conflicting types for '__packed' /home/landley/aboriginal/aboriginal/build/simple-cross-compiler-i686/bin/../include/mtd/ubi-user.h:313:
error: previous declaration of '__packed' was here

This is due to the Linux 3.0 commit in the title:

UBI: use __packed instead of __attribute__((packed))

This relies on __packed being defined in:

  include/linux/compiler-gcc.h:
  #define __packed			__attribute__((packed))

Which is not exported to userspace.

I.E. this "cleanup" broke the ability for userspace to use this header, even though the header is exported to userspace by headers_install.

Rob

Stefan Richter

2011-08-14 18:08 UTC #1 lore raw

Quoted 16 lines · On Aug 14 Rob Landley wrote:

"make headers_install" copies include/mtd/ubi_user.h to userspace, where things like busybox link against it. With 3.0, building defconfig busybox gets this error:
In file included from miscutils/ubi_tools.c:63:
/home/landley/aboriginal/aboriginal/build/simple-cross-compiler-i686/bin/../include/mtd/ubi-user.h:329:
error: conflicting types for '__packed' /home/landley/aboriginal/aboriginal/build/simple-cross-compiler-i686/bin/../include/mtd/ubi-user.h:313:
error: previous declaration of '__packed' was here
This is due to the Linux 3.0 commit in the title:
UBI: use __packed instead of __attribute__((packed))
This relies on __packed being defined in:

Quoted 7 lines

  include/linux/compiler-gcc.h:
  #define __packed			__attribute__((packed))
Which is not exported to userspace.
I.E. this "cleanup" broke the ability for userspace to use this header, even though the header is exported to userspace by headers_install.

As far as I can tell, the __packed or __attribute__((packed)) qualifications are superfluous in include/mtd/ubi-user.h anyway.

Signature · 3 lines

Stefan Richter
-=====-==-== =--- -===-
http://arcgraph.de/sr/

Rob Landley

author

2011-08-14 19:11 UTC #2 lore raw

Quoted 20 lines · Stefan Richter

Quoted 19 lines · On Aug 14 Rob Landley wrote:
"make headers_install" copies include/mtd/ubi_user.h to userspace, where things like busybox link against it. With 3.0, building defconfig busybox gets this error:
In file included from miscutils/ubi_tools.c:63:
/home/landley/aboriginal/aboriginal/build/simple-cross-compiler-i686/bin/../include/mtd/ubi-user.h:329:
error: conflicting types for '__packed' /home/landley/aboriginal/aboriginal/build/simple-cross-compiler-i686/bin/../include/mtd/ubi-user.h:313:
error: previous declaration of '__packed' was here
This is due to the Linux 3.0 commit in the title:
UBI: use __packed instead of __attribute__((packed))
This relies on __packed being defined in:
  include/linux/compiler-gcc.h:
  #define __packed			__attribute__((packed))

Quoted 7 lines

Quoted 4 lines
Which is not exported to userspace.
I.E. this "cleanup" broke the ability for userspace to use this header, even though the header is exported to userspace by headers_install.
As far as I can tell, the __packed or __attribute__((packed)) qualifications are superfluous in include/mtd/ubi-user.h anyway.

And the email address he signed-off-by the broken commit from bounced, trying again with the one out of MAINTAINERS.

Rob

Rob Landley

author

2011-08-14 19:16 UTC #3 lore raw

In reply to Stefan Richter

Quoted 20 lines · Stefan Richter

Quoted 19 lines · On Aug 14 Rob Landley wrote:
"make headers_install" copies include/mtd/ubi_user.h to userspace, where things like busybox link against it. With 3.0, building defconfig busybox gets this error:
In file included from miscutils/ubi_tools.c:63:
/home/landley/aboriginal/aboriginal/build/simple-cross-compiler-i686/bin/../include/mtd/ubi-user.h:329:
error: conflicting types for '__packed' /home/landley/aboriginal/aboriginal/build/simple-cross-compiler-i686/bin/../include/mtd/ubi-user.h:313:
error: previous declaration of '__packed' was here
This is due to the Linux 3.0 commit in the title:
UBI: use __packed instead of __attribute__((packed))
This relies on __packed being defined in:
  include/linux/compiler-gcc.h:
  #define __packed			__attribute__((packed))

Quoted 7 lines

Quoted 4 lines
Which is not exported to userspace.
I.E. this "cleanup" broke the ability for userspace to use this header, even though the header is exported to userspace by headers_install.
As far as I can tell, the __packed or __attribute__((packed)) qualifications are superfluous in include/mtd/ubi-user.h anyway.

And Adrian Hunter <[email protected]> (the other listed UBI maintainer) also bounces. Did the developer exodus after Nokia decided to cozy up to The Black Widow take 'em both out?

Rob

Artem Bityutskiy

2011-08-15 03:39 UTC #4 lore raw

Quoted 26 lines · Rob Landley

Quoted 25 lines · Stefan Richter
Quoted 23 lines · On Aug 14 Rob Landley wrote:
"make headers_install" copies include/mtd/ubi_user.h to userspace, where things like busybox link against it. With 3.0, building defconfig busybox gets this error:
In file included from miscutils/ubi_tools.c:63:
/home/landley/aboriginal/aboriginal/build/simple-cross-compiler-i686/bin/../include/mtd/ubi-user.h:329:
error: conflicting types for '__packed' /home/landley/aboriginal/aboriginal/build/simple-cross-compiler-i686/bin/../include/mtd/ubi-user.h:313:
error: previous declaration of '__packed' was here
This is due to the Linux 3.0 commit in the title:
UBI: use __packed instead of __attribute__((packed))
This relies on __packed being defined in:
  include/linux/compiler-gcc.h:
  #define __packed			__attribute__((packed))
Which is not exported to userspace.
I.E. this "cleanup" broke the ability for userspace to use this header, even though the header is exported to userspace by headers_install.

Quoted 6 lines

Quoted 2 lines
As far as I can tell, the __packed or __attribute__((packed)) qualifications are superfluous in include/mtd/ubi-user.h anyway.
And Adrian Hunter <[email protected]> (the other listed UBI maintainer) also bounces. Did the developer exodus after Nokia decided to cozy up to The Black Widow take 'em both out?

Yes, both of us have left Nokia recently. Adrian should fix the "MAINTAINERS" file.

WRT to the breakage - yes, it was my mistake to follow the new trend in the Linux kernel - the commit message has a reference to the other commit which inspired the check, and you can find out that some other projects made a similar change. Moreover, checkpatch.pl prints the following warning "WARN("__packed is preferred over __attribute__((packed))". So you hardly can blame :-)

Anyways, this problem was discussed in the "__packed vs. __attribute__((packed)) in kernel headers" thread in LKML.

Then a fix has been sent, see the "{PATCH] fix __packed in exported kernel headers" thread in LKML. This fix was accepted by "Michal Marek <[email protected]>", whom I put to CC.

Signature · 2 lines

Best Regards,
Artem Bityutskiy (Битюцкий Артём)

Artem Bityutskiy

2011-08-15 03:42 UTC #5 lore raw

Quoted 44 lines · Artem Bityutskiy

Quoted 26 lines · Rob Landley
Quoted 25 lines · Stefan Richter
Quoted 23 lines · On Aug 14 Rob Landley wrote:
"make headers_install" copies include/mtd/ubi_user.h to userspace, where things like busybox link against it. With 3.0, building defconfig busybox gets this error:
In file included from miscutils/ubi_tools.c:63:
/home/landley/aboriginal/aboriginal/build/simple-cross-compiler-i686/bin/../include/mtd/ubi-user.h:329:
error: conflicting types for '__packed' /home/landley/aboriginal/aboriginal/build/simple-cross-compiler-i686/bin/../include/mtd/ubi-user.h:313:
error: previous declaration of '__packed' was here
This is due to the Linux 3.0 commit in the title:
UBI: use __packed instead of __attribute__((packed))
This relies on __packed being defined in:
  include/linux/compiler-gcc.h:
  #define __packed			__attribute__((packed))
Which is not exported to userspace.
I.E. this "cleanup" broke the ability for userspace to use this header, even though the header is exported to userspace by headers_install.
Quoted 6 lines
Quoted 2 lines
As far as I can tell, the __packed or __attribute__((packed)) qualifications are superfluous in include/mtd/ubi-user.h anyway.
And Adrian Hunter <[email protected]> (the other listed UBI maintainer) also bounces. Did the developer exodus after Nokia decided to cozy up to The Black Widow take 'em both out?
Yes, both of us have left Nokia recently. Adrian should fix the "MAINTAINERS" file.
WRT to the breakage - yes, it was my mistake to follow the new trend in the Linux kernel - the commit message has a reference to the other commit which inspired the check, and you can find out that some other projects made a similar change. Moreover, checkpatch.pl prints the following warning "WARN("__packed is preferred over __attribute__((packed))". So you hardly can blame :-)

Quoted 6 lines

Anyways, this problem was discussed in the "__packed vs. __attribute__((packed)) in kernel headers" thread in LKML.
Then a fix has been sent, see the "{PATCH] fix __packed in exported kernel headers" thread in LKML. This fix was accepted by "Michal Marek <[email protected]>", whom I put to CC.

And the fix is upstream, see:

commit f210735fe2f17a6225432ee3d1239bcf23a8659c

Author: Markus Trippelsdorf <[email protected]>
Date:   Fri Jun 24 15:51:00 2011 +0200

Probably someone can ping the -stable team and ask them to take it to linux-3.0-stable?

Signature · 2 lines

Best Regards,
Artem Bityutskiy (Битюцкий Артём)