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

["Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>]

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