Move the crc and its state into struct gunzip_state. In the process, expand the
only use of CRC_VALUE as it is hides what is being compared.
Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
xen/common/gzip/gunzip.c | 11 +++++++----
xen/common/gzip/inflate.c | 14 +++++---------
2 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/xen/common/gzip/gunzip.c b/xen/common/gzip/gunzip.c
index 0043ff8ac886..26d2a4aa9d36 100644
--- a/xen/common/gzip/gunzip.c
+++ b/xen/common/gzip/gunzip.c
@@ -20,6 +20,9 @@ struct gunzip_state {
unsigned long bb; /* bit buffer */
unsigned int bk; /* bits in bit buffer */
+
+ uint32_t crc_32_tab[256];
+ uint32_t crc;
};
#define malloc(a) xmalloc_bytes(a)
@@ -72,7 +75,7 @@ static __init void flush_window(struct gunzip_state *s)
* The window is equal to the output buffer therefore only need to
* compute the crc.
*/
- unsigned long c = crc;
+ unsigned int c = s->crc;
unsigned int n;
unsigned char *in, ch;
@@ -80,9 +83,9 @@ static __init void flush_window(struct gunzip_state *s)
for ( n = 0; n < s->wp; n++ )
{
ch = *in++;
- c = crc_32_tab[((int)c ^ ch) & 0xff] ^ (c >> 8);
+ c = s->crc_32_tab[((int)c ^ ch) & 0xff] ^ (c >> 8);
}
- crc = c;
+ s->crc = c;
s->bytes_out += (unsigned long)s->wp;
s->wp = 0;
@@ -119,7 +122,7 @@ __init int perform_gunzip(char *output, char *image, unsigned long image_len)
s->inptr = 0;
s->bytes_out = 0;
- makecrc();
+ makecrc(s);
if ( gunzip(s) < 0 )
{
diff --git a/xen/common/gzip/inflate.c b/xen/common/gzip/inflate.c
index 8da14880cfbe..dc940e59d853 100644
--- a/xen/common/gzip/inflate.c
+++ b/xen/common/gzip/inflate.c
@@ -1032,16 +1032,12 @@ static int __init inflate(struct gunzip_state *s)
*
**********************************************************************/
-static ulg __initdata crc_32_tab[256];
-static ulg __initdata crc; /* initialized in makecrc() so it'll reside in bss */
-#define CRC_VALUE (crc ^ 0xffffffffUL)
-
/*
* Code to compute the CRC-32 table. Borrowed from
* gzip-1.0.3/makecrc.c.
*/
-static void __init makecrc(void)
+static void __init makecrc(struct gunzip_state *s)
{
/* Not copyrighted 1990 Mark Adler */
@@ -1058,7 +1054,7 @@ static void __init makecrc(void)
for (i = 0; i < sizeof(p)/sizeof(int); i++)
e |= 1L << (31 - p[i]);
- crc_32_tab[0] = 0;
+ s->crc_32_tab[0] = 0;
for (i = 1; i < 256; i++)
{
@@ -1069,11 +1065,11 @@ static void __init makecrc(void)
if (k & 1)
c ^= e;
}
- crc_32_tab[i] = c;
+ s->crc_32_tab[i] = c;
}
/* this is initialized here so this code could reside in ROM */
- crc = (ulg)0xffffffffUL; /* shift register contents */
+ s->crc = (ulg)~0u; /* shift register contents */
}
/* gzip flag byte */
@@ -1189,7 +1185,7 @@ static int __init gunzip(struct gunzip_state *s)
orig_len |= (ulg) NEXTBYTE(s) << 24;
/* Validate decompression */
- if (orig_crc != CRC_VALUE) {
+ if (orig_crc != (s->crc ^ ~0u)) {
error("crc error");
return -1;
}
--
2.30.2On 24.04.2024 18:34, Daniel P. Smith wrote:
> Move the crc and its state into struct gunzip_state. In the process, expand the
> only use of CRC_VALUE as it is hides what is being compared.
Andrew mentioned uint32_t only for the description, but I think you want
(maybe even need) to go further and actually use that type also, e.g.
> --- a/xen/common/gzip/gunzip.c
> +++ b/xen/common/gzip/gunzip.c
> @@ -20,6 +20,9 @@ struct gunzip_state {
>
> unsigned long bb; /* bit buffer */
> unsigned int bk; /* bits in bit buffer */
> +
> + uint32_t crc_32_tab[256];
> + uint32_t crc;
> };
... not just here, but also ...
> @@ -72,7 +75,7 @@ static __init void flush_window(struct gunzip_state *s)
> * The window is equal to the output buffer therefore only need to
> * compute the crc.
> */
> - unsigned long c = crc;
> + unsigned int c = s->crc;
... here.
> --- a/xen/common/gzip/inflate.c
> +++ b/xen/common/gzip/inflate.c
> @@ -1032,16 +1032,12 @@ static int __init inflate(struct gunzip_state *s)
> *
> **********************************************************************/
>
> -static ulg __initdata crc_32_tab[256];
> -static ulg __initdata crc; /* initialized in makecrc() so it'll reside in bss */
> -#define CRC_VALUE (crc ^ 0xffffffffUL)
Note how this is _not_ same as ~0u, also ...
> @@ -1069,11 +1065,11 @@ static void __init makecrc(void)
> if (k & 1)
> c ^= e;
> }
> - crc_32_tab[i] = c;
> + s->crc_32_tab[i] = c;
> }
>
> /* this is initialized here so this code could reside in ROM */
> - crc = (ulg)0xffffffffUL; /* shift register contents */
> + s->crc = (ulg)~0u; /* shift register contents */
... applicable here then: sizeof(int) >= 4, not ==.
As Andrew indicates, the cast previously wasn't needed here. Keeping it is
at best misleading, when s->crc isn't of that type anymore.
Finally please stick to upper-case number suffixes; see all the related Misra
changes, which (iirc) put in place only upper-case ones.
Jan
On 24/04/2024 5:34 pm, Daniel P. Smith wrote: > Move the crc and its state into struct gunzip_state. In the process, expand the > only use of CRC_VALUE as it is hides what is being compared. "All variables here should be uint32_t rather than unsigned long, which halves the storage space required." > diff --git a/xen/common/gzip/inflate.c b/xen/common/gzip/inflate.c > index 8da14880cfbe..dc940e59d853 100644 > --- a/xen/common/gzip/inflate.c > +++ b/xen/common/gzip/inflate.c > @@ -1069,11 +1065,11 @@ static void __init makecrc(void) > if (k & 1) > c ^= e; > } > - crc_32_tab[i] = c; > + s->crc_32_tab[i] = c; > } > > /* this is initialized here so this code could reside in ROM */ > - crc = (ulg)0xffffffffUL; /* shift register contents */ > + s->crc = (ulg)~0u; /* shift register contents */ The (ulg) wasn't ever necessary, and can be dropped as part of this cleanup. Can fix on commit. ~Andrew
© 2016 - 2026 Red Hat, Inc.