diff mbox series

[v2,5/6] gzip: move crc state into consilidated gzip state

Message ID 20240417143716.27189-6-dpsmith@apertussolutions.com (mailing list archive)
State Superseded
Headers show
Series Clean up of gzip decompressor | expand

Commit Message

Daniel P. Smith April 17, 2024, 2:37 p.m. UTC 
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(-)

Comments

Andrew Cooper April 17, 2024, 5 p.m. UTC | #1 
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
diff mbox series

Patch

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 */