Mailing List Archive

[PATCH v2 5/6] gzip: move crc state into consilidated gzip state
Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
xen/common/gzip/gunzip.c | 11 +++++++----
xen/common/gzip/inflate.c | 12 +++++-------
2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/xen/common/gzip/gunzip.c b/xen/common/gzip/gunzip.c
index 8178a05a0190..bef324d3d166 100644
--- a/xen/common/gzip/gunzip.c
+++ b/xen/common/gzip/gunzip.c
@@ -22,6 +22,9 @@ struct gzip_state {

unsigned long bb; /* bit buffer */
unsigned bk; /* bits in bit buffer */
+
+ uint32_t crc_32_tab[256];
+ uint32_t crc;
};

#define OF(args) args
@@ -74,7 +77,7 @@ static __init void flush_window(struct gzip_state *s)
* The window is equal to the output buffer therefore only need to
* compute the crc.
*/
- unsigned long c = crc;
+ unsigned long c = s->crc;
unsigned int n;
unsigned char *in, ch;

@@ -82,9 +85,9 @@ static __init void flush_window(struct gzip_state *s)
for ( n = 0; n < s->outcnt; 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->outcnt;
s->outcnt = 0;
@@ -121,7 +124,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 5735bbcf7eb4..c18ce20210b0 100644
--- a/xen/common/gzip/inflate.c
+++ b/xen/common/gzip/inflate.c
@@ -1063,16 +1063,14 @@ static int __init inflate(struct gzip_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)
+#define CRC_VALUE (s->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 gzip_state *s)
{
/* Not copyrighted 1990 Mark Adler */

@@ -1089,7 +1087,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++)
{
@@ -1100,11 +1098,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)0xffffffffUL; /* shift register contents */
}

/* gzip flag byte */
--
2.30.2
Re: [PATCH v2 5/6] gzip: move crc state into consilidated gzip state [ In reply to ]
On 17/04/2024 3:37 pm, Daniel P. Smith wrote:
> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>

The change in type is fine, but does need discussing.  Furthermore, ...

> diff --git a/xen/common/gzip/gunzip.c b/xen/common/gzip/gunzip.c
> index 8178a05a0190..bef324d3d166 100644
> --- a/xen/common/gzip/gunzip.c
> +++ b/xen/common/gzip/gunzip.c
> @@ -74,7 +77,7 @@ static __init void flush_window(struct gzip_state *s)
> * The window is equal to the output buffer therefore only need to
> * compute the crc.
> */
> - unsigned long c = crc;
> + unsigned long c = s->crc;

... this wants to be unsigned int I think.

> diff --git a/xen/common/gzip/inflate.c b/xen/common/gzip/inflate.c
> index 5735bbcf7eb4..c18ce20210b0 100644
> --- a/xen/common/gzip/inflate.c
> +++ b/xen/common/gzip/inflate.c
> @@ -1063,16 +1063,14 @@ static int __init inflate(struct gzip_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)
> +#define CRC_VALUE (s->crc ^ 0xffffffffUL)

$ git grep CRC_VALUE
common/gzip/inflate.c:1052:#define CRC_VALUE (s->crc ^ 0xffffffffUL)
common/gzip/inflate.c:1207:    if (orig_crc != CRC_VALUE) {

I'd expand this in it's single user, but like ...

>
> /*
> * 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 gzip_state *s)
> {
> /* Not copyrighted 1990 Mark Adler */
>
> @@ -1089,7 +1087,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++)
> {
> @@ -1100,11 +1098,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)0xffffffffUL; /* shift register contents */

... this, the constant should become -1u or ~0u because of the type change.

I'm not sure what to make of the ROM comment, but I suspect it means the
XOR can be dropped with a bit of care too.

~Andrew