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:06:41 +0000 Message-ID: <53330921.3050309@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> 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 1WSrHp-0005Cj-Ue for xen-devel@lists.xenproject.org; Wed, 26 Mar 2014 17:06:46 +0000 Received: by mail-bk0-f47.google.com with SMTP id w10so633352bkz.20 for ; Wed, 26 Mar 2014 10:06:43 -0700 (PDT) In-Reply-To: <5333119D020000780000291A@nat28.tlf.novell.com> 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 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. -- Julien Grall