Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[PATCH 1/2] inflate_fast: sout is already a short so ptr arith was off by one.

2 views
Skip to first unread message

Joakim Tjernlund

unread,
Feb 28, 2010, 11:20:02 AM2/28/10
to
This won't change anything for current code, but post increment
will break without this fix, should anyone want to try that.

Signed-off-by: Joakim Tjernlund <Joakim.T...@transmode.se>
---
lib/zlib_inflate/inffast.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/lib/zlib_inflate/inffast.c b/lib/zlib_inflate/inffast.c
index fa62fc7..2c13ecc 100644
--- a/lib/zlib_inflate/inffast.c
+++ b/lib/zlib_inflate/inffast.c
@@ -286,7 +286,7 @@ void inflate_fast(z_streamp strm, unsigned start)
} else { /* dist == 1 or dist == 2 */
unsigned short pat16;

- pat16 = *(sout-2+2*OFF);
+ pat16 = *(sout-1+OFF);
if (dist == 1) {
union uu mm;
/* copy one char pattern to both bytes */
--
1.6.4.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Joakim Tjernlund

unread,
Feb 28, 2010, 11:20:01 AM2/28/10
to

Signed-off-by: Joakim Tjernlund <Joakim.T...@transmode.se>
---
lib/crc32.c | 30 ++++++++++++++----------------
1 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/lib/crc32.c b/lib/crc32.c
index 02e3b31..0f45fbf 100644
--- a/lib/crc32.c
+++ b/lib/crc32.c
@@ -30,11 +30,15 @@
#include <asm/atomic.h>
#include "crc32defs.h"
#if CRC_LE_BITS == 8
-#define tole(x) __constant_cpu_to_le32(x)
-#define tobe(x) __constant_cpu_to_be32(x)
+# define tole(x) __constant_cpu_to_le32(x)
#else
-#define tole(x) (x)
-#define tobe(x) (x)
+# define tole(x) (x)
+#endif
+
+#if CRC_BE_BITS == 8
+# define tobe(x) __constant_cpu_to_be32(x)
+#else
+# define tobe(x) (x)
#endif
#include "crc32table.h"

@@ -52,20 +56,19 @@ crc32_body(u32 crc, unsigned char const *buf, size_t len, const u32 *tab)
# else
# define DO_CRC(x) crc = tab[((crc >> 24) ^ (x)) & 255] ^ (crc << 8)
# endif
- const u32 *b = (const u32 *)buf;
+ const u32 *b;
size_t rem_len;

/* Align it */
- if (unlikely((long)b & 3 && len)) {
- u8 *p = (u8 *)b;
+ if (unlikely((long)buf & 3 && len)) {
do {
- DO_CRC(*p++);
- } while ((--len) && ((long)p)&3);
- b = (u32 *)p;
+ DO_CRC(*buf++);
+ } while ((--len) && ((long)buf)&3);
}
rem_len = len & 3;
/* load data 32 bits wide, xor data 32 bits wide. */
len = len >> 2;
+ b = (const u32 *)buf;
for (--b; len; --len) {
crc ^= *++b; /* use pre increment for speed */
DO_CRC(0);
@@ -82,6 +85,7 @@ crc32_body(u32 crc, unsigned char const *buf, size_t len, const u32 *tab)
} while (--len);
}
return crc;
+#undef DO_CRC
}
#endif
/**
@@ -119,9 +123,6 @@ u32 __pure crc32_le(u32 crc, unsigned char const *p, size_t len)
crc = __cpu_to_le32(crc);
crc = crc32_body(crc, p, len, tab);
return __le32_to_cpu(crc);
-#undef ENDIAN_SHIFT
-#undef DO_CRC
-
# elif CRC_LE_BITS == 4
while (len--) {
crc ^= *p++;
@@ -179,9 +180,6 @@ u32 __pure crc32_be(u32 crc, unsigned char const *p, size_t len)
crc = __cpu_to_be32(crc);
crc = crc32_body(crc, p, len, tab);
return __be32_to_cpu(crc);
-#undef ENDIAN_SHIFT
-#undef DO_CRC
-
# elif CRC_BE_BITS == 4
while (len--) {
crc ^= *p++ << 24;

Andrew Morton

unread,
Mar 2, 2010, 6:00:03 PM3/2/10
to
On Sun, 28 Feb 2010 17:06:02 +0100
Joakim Tjernlund <Joakim.T...@transmode.se> wrote:

> This won't change anything for current code, but post increment
> will break without this fix, should anyone want to try that.
>

The description is a bit cryptic. Hopefully you understand what you
mean, but does anyone else?

> diff --git a/lib/zlib_inflate/inffast.c b/lib/zlib_inflate/inffast.c
> index fa62fc7..2c13ecc 100644
> --- a/lib/zlib_inflate/inffast.c
> +++ b/lib/zlib_inflate/inffast.c
> @@ -286,7 +286,7 @@ void inflate_fast(z_streamp strm, unsigned start)
> } else { /* dist == 1 or dist == 2 */
> unsigned short pat16;
>
> - pat16 = *(sout-2+2*OFF);
> + pat16 = *(sout-1+OFF);
> if (dist == 1) {
> union uu mm;
> /* copy one char pattern to both bytes */

The code you're altering was changed two months ago by, err, you. I
don't know if the patch still makes sense in current kernels.

Please don't raise patches against old kernels. Please check that,
redo the patch, add a changelog which helps non-inffast.c people
understand what it does, then resend?

Joakim Tjernlund

unread,
Mar 3, 2010, 2:40:01 AM3/3/10
to
Andrew Morton <ak...@linux-foundation.org> wrote on 2010/03/02 23:52:31:
>
> On Sun, 28 Feb 2010 17:06:02 +0100
> Joakim Tjernlund <Joakim.T...@transmode.se> wrote:
>
> > This won't change anything for current code, but post increment
> > will break without this fix, should anyone want to try that.
> >
>
> The description is a bit cryptic. Hopefully you understand what you
> mean, but does anyone else?

ehh, yes the commit msg is a bit cryptic. I can fix that.

>
> > diff --git a/lib/zlib_inflate/inffast.c b/lib/zlib_inflate/inffast.c
> > index fa62fc7..2c13ecc 100644
> > --- a/lib/zlib_inflate/inffast.c
> > +++ b/lib/zlib_inflate/inffast.c
> > @@ -286,7 +286,7 @@ void inflate_fast(z_streamp strm, unsigned start)
> > } else { /* dist == 1 or dist == 2 */
> > unsigned short pat16;
> >
> > - pat16 = *(sout-2+2*OFF);
> > + pat16 = *(sout-1+OFF);
> > if (dist == 1) {
> > union uu mm;
> > /* copy one char pattern to both bytes */
>
> The code you're altering was changed two months ago by, err, you. I
> don't know if the patch still makes sense in current kernels.
>
> Please don't raise patches against old kernels. Please check that,
> redo the patch, add a changelog which helps non-inffast.c people
> understand what it does, then resend?

Not old, too new actually. You are sitting on a patch from me named
zlib: Make new optimized inflate endian independent
that was deferred to 2.6.34.
This patch is on top of that one.

What should I do now?

Jocke

Joakim Tjernlund

unread,
Mar 3, 2010, 6:40:02 AM3/3/10
to
Andrew Morton <ak...@linux-foundation.org> wrote on 2010/03/03 08:42:17:

>
> On Wed, 3 Mar 2010 08:35:37 +0100 Joakim Tjernlund
> <joakim.t...@transmode.se> wrote:
>
> > Andrew Morton <ak...@linux-foundation.org> wrote on 2010/03/02 23:52:31:
> > >
> > > redo the patch, add a changelog which helps non-inffast.c people
> > > understand what it does, then resend?
> >
> > Not old, too new actually. You are sitting on a patch from me named
> > zlib: Make new optimized inflate endian independent
> > that was deferred to 2.6.34.
> > This patch is on top of that one.
>
> oh, I missed that.

>
> > What should I do now?
>
> Resending is never wrong ;)

How about this then(on top of zlib: Make new optimized inflate endian independent):

From b3d08e29130e0368d4c8eede568c4c7ac4df66ae Mon Sep 17 00:00:00 2001
From: Joakim Tjernlund <Joakim.T...@transmode.se>
Date: Mon, 15 Feb 2010 17:51:58 +0100
Subject: [PATCH 1/2] inflate_fast: sout is already a short so ptr arith was off by one.

inflate_fast() can do either POST INC or PRE INC on its
pointers walking the memory to decompress. Default is
PRE INC.
The sout pointer offset was miscalculated in one case as
the calculation assumed sout was a char *
This breaks inflate_fast() iff configured to do POST INC.

Signed-off-by: Joakim Tjernlund <Joakim.T...@transmode.se>
---
lib/zlib_inflate/inffast.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/lib/zlib_inflate/inffast.c b/lib/zlib_inflate/inffast.c


index fa62fc7..2c13ecc 100644
--- a/lib/zlib_inflate/inffast.c
+++ b/lib/zlib_inflate/inffast.c
@@ -286,7 +286,7 @@ void inflate_fast(z_streamp strm, unsigned start)
} else { /* dist == 1 or dist == 2 */
unsigned short pat16;

- pat16 = *(sout-2+2*OFF);
+ pat16 = *(sout-1+OFF);
if (dist == 1) {
union uu mm;
/* copy one char pattern to both bytes */

--
1.6.4.4

0 new messages