RE: [PATCH] zlib: define get_unaligned16() only when used
From: Arnd Bergmann > Sent: 24 November 2020 11:57 > > On Tue, Nov 24, 2020 at 12:51 PM Christoph Hellwig wrote: > > On Tue, Nov 24, 2020 at 12:08:40PM +0100, Jann Horn wrote: > > > > Since commit acaab7335bd6 ("lib/zlib: remove outdated and incorrect > > > > pre-increment optimization"), get_unaligned16() is only used when > > > > !CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS. > > > > > > > > Hence, make CC=clang W=1 warns: > > > > > > > > lib/zlib_inflate/inffast.c:20:1: > > > > warning: unused function 'get_unaligned16' [-Wunused-function] > > > > > > > > Define get_unaligned16() only when it is actually used. > > > > > > > > Signed-off-by: Lukas Bulwahn > > > > > > AFAICS a nicer option would be to "#include " and > > > then use "get_unaligned", which should automatically do the right > > > thing everywhere and remove the need for defining get_unaligned16() > > > and checking CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS entirely? > > > > Yes, that is the right thing to do. > > It's possible that this didn't work when the code was originally added: > The decompressor functions are called from the compressed boot path, > which is a bit limited regarding which headers it can include, at least > on some architectures. > > I would recommend test-building this for all architectures that include > ../../../../lib/decompress_inflate.c from their boot code. Plausibly it could include a different header that defined the required items for those builds. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Re: [PATCH] zlib: define get_unaligned16() only when used
On Tue, Nov 24, 2020 at 12:56 PM Arnd Bergmann wrote: > > On Tue, Nov 24, 2020 at 12:51 PM Christoph Hellwig wrote: > > On Tue, Nov 24, 2020 at 12:08:40PM +0100, Jann Horn wrote: > > > > Since commit acaab7335bd6 ("lib/zlib: remove outdated and incorrect > > > > pre-increment optimization"), get_unaligned16() is only used when > > > > !CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS. > > > > > > > > Hence, make CC=clang W=1 warns: > > > > > > > > lib/zlib_inflate/inffast.c:20:1: > > > > warning: unused function 'get_unaligned16' [-Wunused-function] > > > > > > > > Define get_unaligned16() only when it is actually used. > > > > > > > > Signed-off-by: Lukas Bulwahn > > > > > > AFAICS a nicer option would be to "#include " and > > > then use "get_unaligned", which should automatically do the right > > > thing everywhere and remove the need for defining get_unaligned16() > > > and checking CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS entirely? > > > > Yes, that is the right thing to do. > > It's possible that this didn't work when the code was originally added: > The decompressor functions are called from the compressed boot path, > which is a bit limited regarding which headers it can include, at least > on some architectures. > > I would recommend test-building this for all architectures that include > ../../../../lib/decompress_inflate.c from their boot code. > Jann, Christoph, Arnd, thanks for the advice. I will start to look into this immediately. Lukas
Re: [PATCH] zlib: define get_unaligned16() only when used
On Tue, Nov 24, 2020 at 12:51 PM Christoph Hellwig wrote: > On Tue, Nov 24, 2020 at 12:08:40PM +0100, Jann Horn wrote: > > > Since commit acaab7335bd6 ("lib/zlib: remove outdated and incorrect > > > pre-increment optimization"), get_unaligned16() is only used when > > > !CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS. > > > > > > Hence, make CC=clang W=1 warns: > > > > > > lib/zlib_inflate/inffast.c:20:1: > > > warning: unused function 'get_unaligned16' [-Wunused-function] > > > > > > Define get_unaligned16() only when it is actually used. > > > > > > Signed-off-by: Lukas Bulwahn > > > > AFAICS a nicer option would be to "#include " and > > then use "get_unaligned", which should automatically do the right > > thing everywhere and remove the need for defining get_unaligned16() > > and checking CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS entirely? > > Yes, that is the right thing to do. It's possible that this didn't work when the code was originally added: The decompressor functions are called from the compressed boot path, which is a bit limited regarding which headers it can include, at least on some architectures. I would recommend test-building this for all architectures that include ../../../../lib/decompress_inflate.c from their boot code. Arnd
Re: [PATCH] zlib: define get_unaligned16() only when used
On Tue, Nov 24, 2020 at 12:08:40PM +0100, Jann Horn wrote: > > Since commit acaab7335bd6 ("lib/zlib: remove outdated and incorrect > > pre-increment optimization"), get_unaligned16() is only used when > > !CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS. > > > > Hence, make CC=clang W=1 warns: > > > > lib/zlib_inflate/inffast.c:20:1: > > warning: unused function 'get_unaligned16' [-Wunused-function] > > > > Define get_unaligned16() only when it is actually used. > > > > Signed-off-by: Lukas Bulwahn > > AFAICS a nicer option would be to "#include " and > then use "get_unaligned", which should automatically do the right > thing everywhere and remove the need for defining get_unaligned16() > and checking CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS entirely? Yes, that is the right thing to do.
Re: [PATCH] zlib: define get_unaligned16() only when used
On Tue, Nov 24, 2020 at 11:40 AM Lukas Bulwahn wrote: > Since commit acaab7335bd6 ("lib/zlib: remove outdated and incorrect > pre-increment optimization"), get_unaligned16() is only used when > !CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS. > > Hence, make CC=clang W=1 warns: > > lib/zlib_inflate/inffast.c:20:1: > warning: unused function 'get_unaligned16' [-Wunused-function] > > Define get_unaligned16() only when it is actually used. > > Signed-off-by: Lukas Bulwahn AFAICS a nicer option would be to "#include " and then use "get_unaligned", which should automatically do the right thing everywhere and remove the need for defining get_unaligned16() and checking CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS entirely?
[PATCH] zlib: define get_unaligned16() only when used
Since commit acaab7335bd6 ("lib/zlib: remove outdated and incorrect pre-increment optimization"), get_unaligned16() is only used when !CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS. Hence, make CC=clang W=1 warns: lib/zlib_inflate/inffast.c:20:1: warning: unused function 'get_unaligned16' [-Wunused-function] Define get_unaligned16() only when it is actually used. Signed-off-by: Lukas Bulwahn --- applies cleanly on current master and next-20201124 Jann, please ack. Andrew, please pick this minor non-urgent clean-up patch. lib/zlib_inflate/inffast.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/zlib_inflate/inffast.c b/lib/zlib_inflate/inffast.c index ed1f3df27260..ca66d9008228 100644 --- a/lib/zlib_inflate/inffast.c +++ b/lib/zlib_inflate/inffast.c @@ -15,7 +15,8 @@ union uu { unsigned char b[2]; }; -/* Endian independed version */ +#ifndef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS +/* Endian independent version */ static inline unsigned short get_unaligned16(const unsigned short *p) { @@ -26,6 +27,7 @@ get_unaligned16(const unsigned short *p) mm.b[1] = b[1]; return mm.us; } +#endif /* Decode literal, length, and distance codes and write out the resulting -- 2.17.1