Message ID | 20140311041707.GA6064@sergelap |
---|---|
State | Committed |
Headers |
Return-Path: <x14307373@homiemail-mx21.g.dreamhost.com> X-Original-To: siddhesh@wilcox.dreamhost.com Delivered-To: siddhesh@wilcox.dreamhost.com Received: from homiemail-mx21.g.dreamhost.com (caibbdcaaahb.dreamhost.com [208.113.200.71]) by wilcox.dreamhost.com (Postfix) with ESMTP id A9A1C36008F for <siddhesh@wilcox.dreamhost.com>; Mon, 10 Mar 2014 21:17:21 -0700 (PDT) Received: by homiemail-mx21.g.dreamhost.com (Postfix, from userid 14307373) id 4521420DB349; Mon, 10 Mar 2014 21:17:21 -0700 (PDT) X-Original-To: glibc@patchwork.siddhesh.in Delivered-To: x14307373@homiemail-mx21.g.dreamhost.com Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by homiemail-mx21.g.dreamhost.com (Postfix) with ESMTPS id 203CC1FDC7FF for <glibc@patchwork.siddhesh.in>; Mon, 10 Mar 2014 21:17:21 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:from:to:cc:subject:message-id :mime-version:content-type; q=dns; s=default; b=TQ3Riafdhd3VR+1T EOtDHLQGPcPu0Thpt8WHApQP7yzvWsTz3CmUscT/fn2YWKWmuRtBE8kjQij4bJdF YtJ6t+v2yDu/MjYWhsh6ON26xUCp1CsZiNFJWqJDbS3mPhneV4XxkYLy0C52CSi9 Ip/ZVafDtXBK/RG3upfIZeEVcJk= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:from:to:cc:subject:message-id :mime-version:content-type; s=default; bh=JJkjTuGvr+kYFvXaUyyJCI XndhQ=; b=Me6jzEjhUfSQtzBWmkl7eJ7hQJZIypBTKdii/zrAW2QIvQxz+q72lz bXjbzeinL9UXGxGFBYJnOTkqfmMui10WuhtQvhiRyq/niynl3Ipo5i5jvIflAKkH i82U4k895brcANBrP25a955/QWS86iRG2Z43hDi4Akv08fhpcMQDw= Received: (qmail 1954 invoked by alias); 11 Mar 2014 04:17:18 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-glibc=patchwork.siddhesh.in@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 1938 invoked by uid 89); 11 Mar 2014 04:17:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00 autolearn=ham version=3.3.2 X-HELO: youngberry.canonical.com Date: Mon, 10 Mar 2014 23:17:07 -0500 From: Serge Hallyn <serge.hallyn@ubuntu.com> To: libc-alpha@sourceware.org Cc: "Andrew G. Morgan" <morgan@kernel.org> Subject: [PATCH] misc/sys/xattr.h: guard against linux uapi header inclusion Message-ID: <20140311041707.GA6064@sergelap> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) X-DH-Original-To: glibc@patchwork.siddhesh.in |
Commit Message
Serge Hallyn
March 11, 2014, 4:17 a.m. UTC
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(+)
Comments
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. */