| Message ID | 20140311041707.GA6064@sergelap |
|---|---|
| State | Committed |
| Headers | show |
On Mon, Mar 10, 2014 at 11:17:07PM -0500, Serge Hallyn wrote: > If the glibc xattr.h header is included after the uapi header, > compilation fails due to an enum re-using a #define from the > uapi header. Protect against this by guarding the define and > enum inclusions against each other. > > (A corresponding kernel patch has been sent here: > http://lkml.org/lkml/2014/3/7/331 ) > > (See https://lists.debian.org/debian-glibc/2014/03/msg00029.html > and https://sourceware.org/glibc/wiki/Synchronizing_Headers > for more information.) > > Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com> > Looks ok, I commited that with appropriate ChangeLog. > --- > misc/sys/xattr.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/misc/sys/xattr.h b/misc/sys/xattr.h > index 929cd87..796df90 100644 > --- a/misc/sys/xattr.h > +++ b/misc/sys/xattr.h > @@ -26,6 +26,7 @@ __BEGIN_DECLS > > /* The following constants should be used for the fifth parameter of > `*setxattr'. */ > +#ifndef __USE_KERNEL_XATTR_DEFS > enum > { > XATTR_CREATE = 1, /* set value, fail if attr already exists. */ > @@ -33,6 +34,7 @@ enum > XATTR_REPLACE = 2 /* set value, fail if attr does not exist. */ > #define XATTR_REPLACE XATTR_REPLACE > }; > +#endif > > /* Set the attribute NAME of the file pointed to by PATH to VALUE (which > is SIZE bytes long). Return 0 on success, -1 for errors. */
On 18/03/14 23:27, Ondřej Bílka wrote: > On Mon, Mar 10, 2014 at 11:17:07PM -0500, Serge Hallyn wrote: >> If the glibc xattr.h header is included after the uapi header, >> compilation fails due to an enum re-using a #define from the >> uapi header. Protect against this by guarding the define and >> enum inclusions against each other. >> >> (A corresponding kernel patch has been sent here: >> http://lkml.org/lkml/2014/3/7/331 ) >> >> (See https://lists.debian.org/debian-glibc/2014/03/msg00029.html >> and https://sourceware.org/glibc/wiki/Synchronizing_Headers >> for more information.) >> >> Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com> >> > Looks ok, I commited that with appropriate ChangeLog. > Has the kernel patch been reviewed? This should not have been committed before there was agreement between both parties that the patches were good. The kernel patch in the link looks wrong to me. The define __UAPI_DEF_XATTR is set to 0 whenever any glibc header is included, not just sys/xattr.h. It should be wrapped in "#ifdef _SYS_XATTR_H". The glibc patch is fine, but pointless until is the kernel patch is accepted. >> --- >> misc/sys/xattr.h | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/misc/sys/xattr.h b/misc/sys/xattr.h >> index 929cd87..796df90 100644 >> --- a/misc/sys/xattr.h >> +++ b/misc/sys/xattr.h >> @@ -26,6 +26,7 @@ __BEGIN_DECLS >> >> /* The following constants should be used for the fifth parameter of >> `*setxattr'. */ >> +#ifndef __USE_KERNEL_XATTR_DEFS >> enum >> { >> XATTR_CREATE = 1, /* set value, fail if attr already exists. */ >> @@ -33,6 +34,7 @@ enum >> XATTR_REPLACE = 2 /* set value, fail if attr does not exist. */ >> #define XATTR_REPLACE XATTR_REPLACE >> }; >> +#endif >> >> /* Set the attribute NAME of the file pointed to by PATH to VALUE (which >> is SIZE bytes long). Return 0 on success, -1 for errors. */ > > >
Quoting Allan McRae (allan@archlinux.org): > On 18/03/14 23:27, Ondřej Bílka wrote: > > On Mon, Mar 10, 2014 at 11:17:07PM -0500, Serge Hallyn wrote: > >> If the glibc xattr.h header is included after the uapi header, > >> compilation fails due to an enum re-using a #define from the > >> uapi header. Protect against this by guarding the define and > >> enum inclusions against each other. > >> > >> (A corresponding kernel patch has been sent here: > >> http://lkml.org/lkml/2014/3/7/331 ) > >> > >> (See https://lists.debian.org/debian-glibc/2014/03/msg00029.html > >> and https://sourceware.org/glibc/wiki/Synchronizing_Headers > >> for more information.) > >> > >> Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com> > >> > > Looks ok, I commited that with appropriate ChangeLog. > > > > Has the kernel patch been reviewed? This should not have been committed Nope, no feedback yet. > before there was agreement between both parties that the patches were good. > > The kernel patch in the link looks wrong to me. The define > __UAPI_DEF_XATTR is set to 0 whenever any glibc header is included, not > just sys/xattr.h. It should be wrapped in "#ifdef _SYS_XATTR_H". Good point, thanks! I missed that part of the network patch I was emulating. I'll fix it up, resend, and cc: akpm. > The glibc patch is fine, but pointless until is the kernel patch is > accepted. > >> --- > >> misc/sys/xattr.h | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/misc/sys/xattr.h b/misc/sys/xattr.h > >> index 929cd87..796df90 100644 > >> --- a/misc/sys/xattr.h > >> +++ b/misc/sys/xattr.h > >> @@ -26,6 +26,7 @@ __BEGIN_DECLS > >> > >> /* The following constants should be used for the fifth parameter of > >> `*setxattr'. */ > >> +#ifndef __USE_KERNEL_XATTR_DEFS > >> enum > >> { > >> XATTR_CREATE = 1, /* set value, fail if attr already exists. */ > >> @@ -33,6 +34,7 @@ enum > >> XATTR_REPLACE = 2 /* set value, fail if attr does not exist. */ > >> #define XATTR_REPLACE XATTR_REPLACE > >> }; > >> +#endif > >> > >> /* Set the attribute NAME of the file pointed to by PATH to VALUE (which > >> is SIZE bytes long). Return 0 on success, -1 for errors. */ > > > > > > >
diff --git a/misc/sys/xattr.h b/misc/sys/xattr.h index 929cd87..796df90 100644 --- a/misc/sys/xattr.h +++ b/misc/sys/xattr.h @@ -26,6 +26,7 @@ __BEGIN_DECLS /* The following constants should be used for the fifth parameter of `*setxattr'. */ +#ifndef __USE_KERNEL_XATTR_DEFS enum { XATTR_CREATE = 1, /* set value, fail if attr already exists. */ @@ -33,6 +34,7 @@ enum XATTR_REPLACE = 2 /* set value, fail if attr does not exist. */ #define XATTR_REPLACE XATTR_REPLACE }; +#endif /* Set the attribute NAME of the file pointed to by PATH to VALUE (which is SIZE bytes long). Return 0 on success, -1 for errors. */
If the glibc xattr.h header is included after the uapi header, compilation fails due to an enum re-using a #define from the uapi header. Protect against this by guarding the define and enum inclusions against each other. (A corresponding kernel patch has been sent here: http://lkml.org/lkml/2014/3/7/331 ) (See https://lists.debian.org/debian-glibc/2014/03/msg00029.html and https://sourceware.org/glibc/wiki/Synchronizing_Headers for more information.) Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com> --- misc/sys/xattr.h | 2 ++ 1 file changed, 2 insertions(+)