Re: [Xen-devel] [PATCH 05/34] xen/xsm: flask: Remove unused function avc_sidcmp

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.03.14 at 17:11, <julien.grall@xxxxxxxxxx> wrote:
> On 03/26/2014 11:57 AM, Jan Beulich wrote:
>>>>> On 25.03.14 at 17:55, <julien.grall@xxxxxxxxxx> 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).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel