Message ID | 1478695312-5009-1-git-send-email-adhemerval.zanella@linaro.org |
---|---|
State | Dropped |
Headers |
Received: (qmail 101796 invoked by alias); 9 Nov 2016 12:42:13 -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-##L=##H@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 101763 invoked by uid 89); 9 Nov 2016 12:42:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=no version=3.3.2 spammy=sh4-linux-gnu, sh4linuxgnu, 17, 9, undefine X-HELO: mail-ua0-f182.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:subject:date:message-id; bh=PTaxS2UxzZsNDSV4heYyiKVq0Zibnzu1bVvtbKKpF0Q=; b=Dt6GRQI318svMfqZpe7R9T3jpvZtDi/p1Hg6IflDVifOE+rO+XJdRhVzo8QPMkrRfi rSVUw5Hrmz0lGLp5WaI30d/IZb2cisxqqoYM6zGm7h1HAHVzx5Z66E5WIWz+AWINc6nM T72bIJfK00LhTVUgyFoHmJv12GdsSgOsxiA7gC3g7IstB0glVJc4ApLjdF7kRm8BEnfx ntDllwKVTyZ0RvJe2SBLt8kZgDFpke0xd+CPFMTM47QBtU20YicE1W7niePiP/2TfkY8 JvXPBtQHliNrOq5LMhEadFe3g0zM6ANeORAbjvLnElcW+Nud9WGCvp42ttvd49FHsc8Y NkPw== X-Gm-Message-State: ABUngvdjxzNFtEzuvZxsOJPbinCdynF262jcxzpOOTrSNqzUBmc9yaJiEzzrjl5Bi1AzZqsm X-Received: by 10.159.33.97 with SMTP id 88mr420178uab.156.1478695319016; Wed, 09 Nov 2016 04:41:59 -0800 (PST) From: Adhemerval Zanella <adhemerval.zanella@linaro.org> To: libc-alpha@sourceware.org Subject: [PATCH] Fix sh4 build with __ASSUME_ST_INO_64_BIT redefinition Date: Wed, 9 Nov 2016 10:41:52 -0200 Message-Id: <1478695312-5009-1-git-send-email-adhemerval.zanella@linaro.org> |
Commit Message
Adhemerval Zanella Netto
Nov. 9, 2016, 12:41 p.m. UTC
Since 327792c sh4 builds fails with: ../sysdeps/unix/sysv/linux/kernel-features.h:49:0: error: "__ASSUME_ST_INO_64_BIT" redefined [-Werror] #define __ASSUME_ST_INO_64_BIT 1 ^ In file included from ../sysdeps/unix/sysv/linux/sysdep.h:19:0, from ../sysdeps/unix/sysv/linux/sh/sysdep.h:24, from ../sysdeps/unix/sysv/linux/sh/sh4/sysdep.h:4, from <stdin>:1: ../sysdeps/unix/sysv/linux/sh/kernel-features.h:47:0: note: this is the location of the previous definition #define __ASSUME_ST_INO_64_BIT 0 It is because sh4 kernel-features.sh is included multiple times without guards and this patch fixes by adding them. Tested on a sh4-linux-gnu build. Also with this issues, is there are strong reason to *not* have include guards on kernel-features.h? With current approach, a architecture can't include Linux default kernel-features.h and redefine it to a different value, only undefine it (unless it explicit do not include default kernel-features.h, and I think that's is not the idea). * sysdeps/unix/sysv/linux/sh/kernel-features.h: Add include guards. --- ChangeLog | 5 +++++ sysdeps/unix/sysv/linux/sh/kernel-features.h | 5 +++++ 2 files changed, 10 insertions(+)
Comments
On Wed, 9 Nov 2016, Adhemerval Zanella wrote: > It is because sh4 kernel-features.sh is included multiple times > without guards and this patch fixes by adding them. Tested on a > sh4-linux-gnu build. > > Also with this issues, is there are strong reason to *not* have include guards > on kernel-features.h? With current approach, a architecture can't include > Linux default kernel-features.h and redefine it to a different value, only > undefine it (unless it explicit do not include default kernel-features.h, > and I think that's is not the idea). > > * sysdeps/unix/sysv/linux/sh/kernel-features.h: Add include > guards. OK. I think it's appropriate for all these headers to have include guards. The #ifndef, #undef etc. could be avoided (moving this header closer to glibc's macro-name-typo-proof norms in this regard) if the header were split up into smaller pieces for individual macros or groups of related macros whenever a macro's definition needs to vary between architectures. Thus, you'd have e.g. kernel-features-st-ino.h defining __ASSUME_ST_INO_64_BIT to either 0 or 1, the default version of that header would define to 0 and the architectures overriding it would define to 1 (and wouldn't need #include_next). kernel-features.h could end up as an architecture-independent header that just #includes the smaller architecture-specific headers. (This is not a full design, especially for the more complicated cases such as socket syscalls.)
On 10/11/2016 16:07, Joseph Myers wrote: > On Wed, 9 Nov 2016, Adhemerval Zanella wrote: > >> It is because sh4 kernel-features.sh is included multiple times >> without guards and this patch fixes by adding them. Tested on a >> sh4-linux-gnu build. >> >> Also with this issues, is there are strong reason to *not* have include guards >> on kernel-features.h? With current approach, a architecture can't include >> Linux default kernel-features.h and redefine it to a different value, only >> undefine it (unless it explicit do not include default kernel-features.h, >> and I think that's is not the idea). >> >> * sysdeps/unix/sysv/linux/sh/kernel-features.h: Add include >> guards. > > OK. I think it's appropriate for all these headers to have include > guards. Ok, I will push this patch. > > The #ifndef, #undef etc. could be avoided (moving this header closer to > glibc's macro-name-typo-proof norms in this regard) if the header were > split up into smaller pieces for individual macros or groups of related > macros whenever a macro's definition needs to vary between architectures. > Thus, you'd have e.g. kernel-features-st-ino.h defining > __ASSUME_ST_INO_64_BIT to either 0 or 1, the default version of that > header would define to 0 and the architectures overriding it would define > to 1 (and wouldn't need #include_next). kernel-features.h could end up as > an architecture-independent header that just #includes the smaller > architecture-specific headers. (This is not a full design, especially for > the more complicated cases such as socket syscalls.) This could be a feasible approach, although it would require some extensive refactoring. But I think we current approach, where architecture kernel-feature is used from sysdep and it is responsible to include default linux one and undef and redefine value, plus guards should be also feasible for macro-name-typo-proof as well.
diff --git a/sysdeps/unix/sysv/linux/sh/kernel-features.h b/sysdeps/unix/sysv/linux/sh/kernel-features.h index ea4fdbc..d03aafa 100644 --- a/sysdeps/unix/sysv/linux/sh/kernel-features.h +++ b/sysdeps/unix/sysv/linux/sh/kernel-features.h @@ -17,6 +17,9 @@ License along with the GNU C Library; if not, see <http://www.gnu.org/licenses/>. */ +#ifndef __KERNEL_FEATURES_SH__ +# define __KERNEL_FEATURES_SH__ + /* SH uses socketcall. */ #define __ASSUME_SOCKETCALL 1 @@ -50,3 +53,5 @@ the kernel interface for p{read,write}64 adds a dummy long argument before the offset. */ #define __ASSUME_PRW_DUMMY_ARG 1 + +#endif