From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH 05/34] xen/xsm: flask: Remove unused function avc_sidcmp Date: Wed, 26 Mar 2014 17:30:41 +0000 Message-ID: <53330EC1.7020508@linaro.org> References: <1395766541-23979-1-git-send-email-julien.grall@linaro.org> <1395766541-23979-6-git-send-email-julien.grall@linaro.org> <5332CEC10200007800002493@nat28.tlf.novell.com> <5332FC43.8050806@linaro.org> <5333119D020000780000291A@nat28.tlf.novell.com> <53330921.3050309@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WSrf4-0006z8-Ik for xen-devel@lists.xenproject.org; Wed, 26 Mar 2014 17:30:46 +0000 Received: by mail-bk0-f44.google.com with SMTP id mz13so647496bkb.31 for ; Wed, 26 Mar 2014 10:30:44 -0700 (PDT) In-Reply-To: <53330921.3050309@linaro.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel@lists.xenproject.org, Daniel De Graaf , stefano.stabellini@citrix.com, ian.campbell@citrix.com, tim@xen.org List-Id: xen-devel@lists.xenproject.org On 03/26/2014 05:06 PM, Julien Grall wrote: > On 03/26/2014 04:42 PM, Jan Beulich wrote: >>>>> On 26.03.14 at 17:11, wrote: >>> On 03/26/2014 11:57 AM, Jan Beulich wrote: >>>>>>> On 25.03.14 at 17:55, wrote: >>>>> Fix compilation with Clang 3.5: >>>>> >>>>> avc.c:657:19: error: unused function 'avc_sidcmp' [-Werror,-Wunused-function] >>>>> static inline int avc_sidcmp(u32 x, u32 y) >>>> >>>> Despite Daniel having acked this already, this seems conceptually wrong >>>> to me: static inline functions are quite frequently unused (and I assume >>>> the compiler warns about them only if they're not in a header file), and >>>> hence the compiler shouldn't be warning about them in the first place. >>> >>> This function has not been used seen 2007. I think this is the only >>> static inline function not defined in headers which is not used. >>> >>> Why shouldn't the compiler warn about it? Rather than static inline >>> function defined in the header, this kind function is dead code. If we >>> want to keep it we should at least mark them as unused. >>> >>> IHMO I don't think we need to keep function that weren't used since ages >>> and I bet it was a forgotten when the code was reworked a long time ago. >> >> Yes, and my comment wasn't about this specific function, but the >> general pattern: You justified your change with the build breakage, >> whereas you could have stated what you said just now in your >> reply. IOW I'm fine with you cleaning up things that were more or >> less obviously forgotten in an earlier change. But code adjustments >> just to satisfy an overly picky compiler aren't that nice. After all >> such functions can serve a purpose (and I think if you looked around >> you'd find other unused stuff that could be deleted yet - so far - isn't >> being, as being potentially useful in the future), and the compiler here >> - from what I can tell - is warning on these simply because they aren't >> in header files. Which in turn raises the question how the compiler >> knows what a header file is (remember that we have quite a few >> instances of .c files including other .c files, and I'd bet the compiler >> treats these as header files too). Summary: Likely the compiler is >> issuing this sort of warning inconsistently, and hence it would better >> not issue the warning at all (or at least provide a way to suppress it). > > Thanks for your comment. I will update the different commit message. > > The new version of clang has amazing feature like guard checking (see > patch #17)... I was wondering the same thing when I first discovered > this kind of errors. > > I guess with the '# 1 "/local/.../.h" hints the compiler is able to know > where does the error come from. > > I will try to find why clang is considering static inline invalid in .c > context. > Here we go, this has been explicitly added in clang a while ago: commit 39bd371610af27b073c792c54c6c28133329f6ad Date: Tue Sep 10 03:05:56 2013 +0000 Make -Wunused warning rules more consistent. This patch does a few different things. This patch improves unused var diags for const vars: we no longer unconditionally suppress diagnostics for const vars, instead only suppressing the diagnostic when the declaration appears to be useful. This patch also makes us more consistently use whether a variable/function is declared in the main file to suppress diagnostics where appropriate. Fixes . git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@190382 91177308-0d34-0410-b5e6-96231b3b80d8 -- Julien Grall