diff mbox series

[4/5] gzip: move crc state into consilidated gzip state

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

Commit Message

Daniel P. Smith April 11, 2024, 3:25 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 11, 2024, 7:43 p.m. UTC | #1 
On 11/04/2024 4:25 pm, Daniel P. Smith wrote:
> diff --git a/xen/common/gzip/inflate.c b/xen/common/gzip/inflate.c
> index c8dd35962abb..6c8c7452a31f 100644
> --- a/xen/common/gzip/inflate.c
> +++ b/xen/common/gzip/inflate.c
> @@ -1125,16 +1125,14 @@ static int __init inflate(struct gzip_data *gd)
>   *
>   **********************************************************************/
>  
> -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 (gd->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_data *gd)
>  {
>  /* Not copyrighted 1990 Mark Adler */
>  
> @@ -1151,7 +1149,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;
> +    gd->crc_32_tab[0] = 0;
>  
>      for (i = 1; i < 256; i++)
>      {
> @@ -1162,11 +1160,11 @@ static void __init makecrc(void)
>              if (k & 1)
>                  c ^= e;
>          }
> -        crc_32_tab[i] = c;
> +        gd->crc_32_tab[i] = c;
>      }
>  
>      /* this is initialized here so this code could reside in ROM */
> -    crc = (ulg)0xffffffffUL; /* shift register contents */
> +    gd->crc = (ulg)0xffffffffUL; /* shift register contents */
>  }
>  
>  /* gzip flag byte */

I can't see any way that a non-32bit value ever gets stored, because 'e'
doesn't ever have bit 32 set in it.  I have a sneaking suspicion that
this is code written in the 32bit days, where sizeof(long) was still 4.

This change, if correct, halves the size of gzip_state.

~Andrew
Daniel P. Smith April 12, 2024, 11:42 a.m. UTC | #2 
On 4/11/24 15:43, Andrew Cooper wrote:
> On 11/04/2024 4:25 pm, Daniel P. Smith wrote:
>> diff --git a/xen/common/gzip/inflate.c b/xen/common/gzip/inflate.c
>> index c8dd35962abb..6c8c7452a31f 100644
>> --- a/xen/common/gzip/inflate.c
>> +++ b/xen/common/gzip/inflate.c
>> @@ -1125,16 +1125,14 @@ static int __init inflate(struct gzip_data *gd)
>>    *
>>    **********************************************************************/
>>   
>> -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 (gd->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_data *gd)
>>   {
>>   /* Not copyrighted 1990 Mark Adler */
>>   
>> @@ -1151,7 +1149,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;
>> +    gd->crc_32_tab[0] = 0;
>>   
>>       for (i = 1; i < 256; i++)
>>       {
>> @@ -1162,11 +1160,11 @@ static void __init makecrc(void)
>>               if (k & 1)
>>                   c ^= e;
>>           }
>> -        crc_32_tab[i] = c;
>> +        gd->crc_32_tab[i] = c;
>>       }
>>   
>>       /* this is initialized here so this code could reside in ROM */
>> -    crc = (ulg)0xffffffffUL; /* shift register contents */
>> +    gd->crc = (ulg)0xffffffffUL; /* shift register contents */
>>   }
>>   
>>   /* gzip flag byte */
> 
> I can't see any way that a non-32bit value ever gets stored, because 'e'
> doesn't ever have bit 32 set in it.  I have a sneaking suspicion that
> this is code written in the 32bit days, where sizeof(long) was still 4.

Yes, I can switch crc and crc_32_tab to uint32_t.

> This change, if correct, halves the size of gzip_state.

Ack.

v/r,
dps
diff mbox series

Patch

diff --git a/xen/common/gzip/gunzip.c b/xen/common/gzip/gunzip.c
index 9b4891731b8b..a1b516b925c9 100644
--- a/xen/common/gzip/gunzip.c
+++ b/xen/common/gzip/gunzip.c
@@ -28,6 +28,9 @@  struct gzip_data {
 
     unsigned long bb;      /* bit buffer */
     unsigned bk;           /* bits in bit buffer */
+
+    unsigned long crc_32_tab[256];
+    unsigned long crc;
 };
 
 #define OF(args)        args
@@ -78,7 +81,7 @@  static __init void flush_window(struct gzip_data *gd)
      * The window is equal to the output buffer therefore only need to
      * compute the crc.
      */
-    unsigned long c = crc;
+    unsigned long c = gd->crc;
     unsigned int n;
     unsigned char *in, ch;
 
@@ -86,9 +89,9 @@  static __init void flush_window(struct gzip_data *gd)
     for ( n = 0; n < gd->outcnt; n++ )
     {
         ch = *in++;
-        c = crc_32_tab[((int)c ^ ch) & 0xff] ^ (c >> 8);
+        c = gd->crc_32_tab[((int)c ^ ch) & 0xff] ^ (c >> 8);
     }
-    crc = c;
+    gd->crc = c;
 
     gd->bytes_out += (unsigned long)gd->outcnt;
     gd->outcnt = 0;
@@ -129,7 +132,7 @@  __init int perform_gunzip(char *output, char *image, unsigned long image_len)
     gd.inptr = 0;
     gd.bytes_out = 0;
 
-    makecrc();
+    makecrc(&gd);
 
     if ( gunzip(&gd) < 0 )
     {
diff --git a/xen/common/gzip/inflate.c b/xen/common/gzip/inflate.c
index c8dd35962abb..6c8c7452a31f 100644
--- a/xen/common/gzip/inflate.c
+++ b/xen/common/gzip/inflate.c
@@ -1125,16 +1125,14 @@  static int __init inflate(struct gzip_data *gd)
  *
  **********************************************************************/
 
-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 (gd->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_data *gd)
 {
 /* Not copyrighted 1990 Mark Adler */
 
@@ -1151,7 +1149,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;
+    gd->crc_32_tab[0] = 0;
 
     for (i = 1; i < 256; i++)
     {
@@ -1162,11 +1160,11 @@  static void __init makecrc(void)
             if (k & 1)
                 c ^= e;
         }
-        crc_32_tab[i] = c;
+        gd->crc_32_tab[i] = c;
     }
 
     /* this is initialized here so this code could reside in ROM */
-    crc = (ulg)0xffffffffUL; /* shift register contents */
+    gd->crc = (ulg)0xffffffffUL; /* shift register contents */
 }
 
 /* gzip flag byte */