Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1647404 > unrolled thread

[PATCH] zlib: Put get_unaligned16() inside #ifdef block

Back to article view | Back to linux.kernel

Contents

  [PATCH] zlib: Put get_unaligned16() inside #ifdef block Matthias Kaehlcke <mka@chromium.org> - 2017-05-22 23:20 +0200
    Re: [PATCH] zlib: Put get_unaligned16() inside #ifdef block Andrew Morton <akpm@linux-foundation.org> - 2017-05-22 23:40 +0200
      Re: [PATCH] zlib: Put get_unaligned16() inside #ifdef block Matthias Kaehlcke <mka@chromium.org> - 2017-05-23 00:20 +0200
    Re: [PATCH] zlib: Put get_unaligned16() inside #ifdef block Christoph Hellwig <hch@infradead.org> - 2017-05-23 09:40 +0200
      Re: [PATCH] zlib: Put get_unaligned16() inside #ifdef block Matthias Kaehlcke <mka@chromium.org> - 2017-05-23 20:10 +0200

#1647404 — [PATCH] zlib: Put get_unaligned16() inside #ifdef block

The function is not used when CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS=y.
Adding the #ifdef fixes the following warning when building with clang:

lib/zlib_inflate/inffast.c:31:1: error: unused function 'get_unaligned16'
    [-Werror,-Wunused-function]

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
Note: Usually we would use the __maybe_unused attribute to silence the
warning. Since this code is used in the kernel decompression stub rather
than in the kernel itself we can't include <linux/compiler.h> with the
definition of __maybe_unused (it would be possible for some platforms,
however for powerpc the build fails with a compiler error). We could
redefine __maybe_unused or use the raw __attribute__((unused)), but
using the #ifdef is a simpler solution.

 lib/zlib_inflate/inffast.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/zlib_inflate/inffast.c b/lib/zlib_inflate/inffast.c
index 2c13ecc5bb2c..af2fd95e35e9 100644
--- a/lib/zlib_inflate/inffast.c
+++ b/lib/zlib_inflate/inffast.c
@@ -26,6 +26,7 @@ union uu {
 	unsigned char b[2];
 };
 
+#ifndef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
 /* Endian independed version */
 static inline unsigned short
 get_unaligned16(const unsigned short *p)
@@ -37,6 +38,7 @@ get_unaligned16(const unsigned short *p)
 	mm.b[1] = b[1];
 	return mm.us;
 }
+#endif
 
 #ifdef POSTINC
 #  define OFF 0
-- 
2.13.0.303.g4ebf302169-goog

[toc] | [next] | [standalone]

#1647415

On Mon, 22 May 2017 14:13:26 -0700 Matthias Kaehlcke <mka@chromium.org> wrote:

> The function is not used when CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS=y.
> Adding the #ifdef fixes the following warning when building with clang:
> 
> lib/zlib_inflate/inffast.c:31:1: error: unused function 'get_unaligned16'
>     [-Werror,-Wunused-function]
> 
> ...
>
> --- a/lib/zlib_inflate/inffast.c
> +++ b/lib/zlib_inflate/inffast.c
> @@ -26,6 +26,7 @@ union uu {
>  	unsigned char b[2];
>  };
>  
> +#ifndef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
>  /* Endian independed version */
>  static inline unsigned short
>  get_unaligned16(const unsigned short *p)
> @@ -37,6 +38,7 @@ get_unaligned16(const unsigned short *p)
>  	mm.b[1] = b[1];
>  	return mm.us;
>  }
> +#endif
>  
>  #ifdef POSTINC
>  #  define OFF 0

Do we really want to mucky up the source code to keep clang happy?  gcc
won't warn about an unused static inline.  Can we configure clang to do
the same?

[toc] | [prev] | [next] | [standalone]

#1647438

Hi Andrew,

El Mon, May 22, 2017 at 02:39:25PM -0700 Andrew Morton ha dit:

> On Mon, 22 May 2017 14:13:26 -0700 Matthias Kaehlcke <mka@chromium.org> wrote:
> 
> > The function is not used when CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS=y.
> > Adding the #ifdef fixes the following warning when building with clang:
> > 
> > lib/zlib_inflate/inffast.c:31:1: error: unused function 'get_unaligned16'
> >     [-Werror,-Wunused-function]
> > 
> > ...
> >
> > --- a/lib/zlib_inflate/inffast.c
> > +++ b/lib/zlib_inflate/inffast.c
> > @@ -26,6 +26,7 @@ union uu {
> >  	unsigned char b[2];
> >  };
> >  
> > +#ifndef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
> >  /* Endian independed version */
> >  static inline unsigned short
> >  get_unaligned16(const unsigned short *p)
> > @@ -37,6 +38,7 @@ get_unaligned16(const unsigned short *p)
> >  	mm.b[1] = b[1];
> >  	return mm.us;
> >  }
> > +#endif
> >  
> >  #ifdef POSTINC
> >  #  define OFF 0
> 
> Do we really want to mucky up the source code to keep clang happy?  gcc
> won't warn about an unused static inline.  Can we configure clang to do
> the same?

To my knowledge there is no option to tell clang to behave like gcc in
this aspect.

On the positive side the number of instances is relatively limited, at
least for defconfig and my custom configs. In most cases it is enough
with adding __maybe_unused, which is already widely used in the kernel.

Personally I think it is useful to be warned about unused functions in
.c files, regardless of whether the function is inline or not.

[toc] | [prev] | [next] | [standalone]

#1647732

On Mon, May 22, 2017 at 02:13:26PM -0700, Matthias Kaehlcke wrote:
> The function is not used when CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS=y.
> Adding the #ifdef fixes the following warning when building with clang:
> 
> lib/zlib_inflate/inffast.c:31:1: error: unused function 'get_unaligned16'
>     [-Werror,-Wunused-function]
> 
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> Note: Usually we would use the __maybe_unused attribute to silence the
> warning. Since this code is used in the kernel decompression stub rather
> than in the kernel itself we can't include <linux/compiler.h> with the
> definition of __maybe_unused (it would be possible for some platforms,
> however for powerpc the build fails with a compiler error). We could
> redefine __maybe_unused or use the raw __attribute__((unused)), but
> using the #ifdef is a simpler solution.

Usually one would take a look at the root cause.  And then remove
get_unaligned16 entirely and replace it with the get_unaligned
helper provided by the kernel.

[toc] | [prev] | [next] | [standalone]

#1648316

Hi Christoph,

El Tue, May 23, 2017 at 12:36:01AM -0700 Christoph Hellwig ha dit:

> On Mon, May 22, 2017 at 02:13:26PM -0700, Matthias Kaehlcke wrote:
> > The function is not used when CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS=y.
> > Adding the #ifdef fixes the following warning when building with clang:
> > 
> > lib/zlib_inflate/inffast.c:31:1: error: unused function 'get_unaligned16'
> >     [-Werror,-Wunused-function]
> > 
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> > Note: Usually we would use the __maybe_unused attribute to silence the
> > warning. Since this code is used in the kernel decompression stub rather
> > than in the kernel itself we can't include <linux/compiler.h> with the
> > definition of __maybe_unused (it would be possible for some platforms,
> > however for powerpc the build fails with a compiler error). We could
> > redefine __maybe_unused or use the raw __attribute__((unused)), but
> > using the #ifdef is a simpler solution.
> 
> Usually one would take a look at the root cause.  And then remove
> get_unaligned16 entirely and replace it with the get_unaligned
> helper provided by the kernel.

Thanks for the suggestion to use get_unaligned().

Unfortunately that doesn't work without hackery, at least for
powerpc. We can add arch/powerpc/include to the include paths of
the decompression stub to be able to include <asm/unaligned.h>,
however the build then fails with "undefined reference to
`get_unaligned'", because on powerpc get/put_unaligned are defined
inside an #ifdef __KERNEL__ block. We probably don't want to define
__KERNEL__ for the decompression stub just to avoid an #ifndef in the
zlib code.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web