Re: [PATCH v3 7/8] gzip: move bitbuffer into gunzip state
On 26/04/2024 6:57 am, Jan Beulich wrote:
> On 26.04.2024 07:55, Jan Beulich wrote:
>> On 25.04.2024 21:23, Andrew Cooper wrote:
>>> On 24/04/2024 5:34 pm, Daniel P. Smith wrote:
--- a/xen/common/gzip/inflate.c
+++ b/xen/common/gzip/inflate.c
@@ -1017,8 +1014,8 @@ static int __init inflate(struct gunzip_state *s)
/* Undo too much lookahead. The next read will be byte aligned so we
* can discard unused bits in the last meaningful byte.
*/
-while (bk >= 8) {
-bk -= 8;
+while (s->bk >= 8) {
+s->bk -= 8;
s->inptr--;
}
>>> Isn't it just me, but isn't this just:
>>>
>>> s->inptr -= (s->bk >> 3);
>>> s->bk &= 7;
>>>
>>> ?
>> I'd say yes, if only there wasn't the comment talking of just a single byte,
>> and even there supposedly some of the bits.
The comments in this file leave a lot to be desired...
I'm reasonably confident they were not adjusted when a piece of
userspace code was imported into Linux.
This cannot ever have been just a byte's worth of bits, seeing as the
bit count is from 8 or more. Right now it's an unsigned long's worth of
bits.
> Talking of the comment: Considering what patch 1 supposedly does, how come
> that isn't Xen-style (yet)?
I had been collecting some minor fixes like this to fold into patch 1
when I committed the whole series.
I'll see if I can fold them elsewhere.
~Andrew
Re: [PATCH v3 7/8] gzip: move bitbuffer into gunzip state
On 26.04.2024 07:55, Jan Beulich wrote:
> On 25.04.2024 21:23, Andrew Cooper wrote:
>> On 24/04/2024 5:34 pm, Daniel P. Smith wrote:
>>> --- a/xen/common/gzip/inflate.c
>>> +++ b/xen/common/gzip/inflate.c
>>> @@ -1017,8 +1014,8 @@ static int __init inflate(struct gunzip_state *s)
>>> /* Undo too much lookahead. The next read will be byte aligned so we
>>> * can discard unused bits in the last meaningful byte.
>>> */
>>> -while (bk >= 8) {
>>> -bk -= 8;
>>> +while (s->bk >= 8) {
>>> +s->bk -= 8;
>>> s->inptr--;
>>> }
>>
>> Isn't it just me, but isn't this just:
>>
>> s->inptr -= (s->bk >> 3);
>> s->bk &= 7;
>>
>> ?
>
> I'd say yes, if only there wasn't the comment talking of just a single byte,
> and even there supposedly some of the bits.
Talking of the comment: Considering what patch 1 supposedly does, how come
that isn't Xen-style (yet)?
Jan
Re: [PATCH v3 7/8] gzip: move bitbuffer into gunzip state
On 25.04.2024 21:23, Andrew Cooper wrote:
> On 24/04/2024 5:34 pm, Daniel P. Smith wrote:
>> --- a/xen/common/gzip/inflate.c
>> +++ b/xen/common/gzip/inflate.c
>> @@ -1017,8 +1014,8 @@ static int __init inflate(struct gunzip_state *s)
>> /* Undo too much lookahead. The next read will be byte aligned so we
>> * can discard unused bits in the last meaningful byte.
>> */
>> -while (bk >= 8) {
>> -bk -= 8;
>> +while (s->bk >= 8) {
>> +s->bk -= 8;
>> s->inptr--;
>> }
>
> Isn't it just me, but isn't this just:
>
> s->inptr -= (s->bk >> 3);
> s->bk &= 7;
>
> ?
I'd say yes, if only there wasn't the comment talking of just a single byte,
and even there supposedly some of the bits.
Jan
Re: [PATCH v3 7/8] gzip: move bitbuffer into gunzip state
On 24/04/2024 5:34 pm, Daniel P. Smith wrote:
> Signed-off-by: Daniel P. Smith
Acked-by: Andrew Cooper
> diff --git a/xen/common/gzip/inflate.c b/xen/common/gzip/inflate.c
> index bec8801df487..8da14880cfbe 100644
> --- a/xen/common/gzip/inflate.c
> +++ b/xen/common/gzip/inflate.c
> @@ -1017,8 +1014,8 @@ static int __init inflate(struct gunzip_state *s)
> /* Undo too much lookahead. The next read will be byte aligned so we
> * can discard unused bits in the last meaningful byte.
> */
> -while (bk >= 8) {
> -bk -= 8;
> +while (s->bk >= 8) {
> +s->bk -= 8;
> s->inptr--;
> }
Isn't it just me, but isn't this just:
s->inptr -= (s->bk >> 3);
s->bk &= 7;
?
~Andrew
[PATCH v3 7/8] gzip: move bitbuffer into gunzip state
Signed-off-by: Daniel P. Smith
---
xen/common/gzip/gunzip.c | 3 +++
xen/common/gzip/inflate.c | 43 ++-
2 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/xen/common/gzip/gunzip.c b/xen/common/gzip/gunzip.c
index 95d924d36726..0043ff8ac886 100644
--- a/xen/common/gzip/gunzip.c
+++ b/xen/common/gzip/gunzip.c
@@ -17,6 +17,9 @@ struct gunzip_state {
unsigned int inptr;
unsigned long bytes_out;
+
+unsigned long bb; /* bit buffer */
+unsigned int bk; /* bits in bit buffer */
};
#define malloc(a) xmalloc_bytes(a)
diff --git a/xen/common/gzip/inflate.c b/xen/common/gzip/inflate.c
index bec8801df487..8da14880cfbe 100644
--- a/xen/common/gzip/inflate.c
+++ b/xen/common/gzip/inflate.c
@@ -211,9 +211,6 @@ static const ush cpdext[] = { /* Extra bits for
distance codes */
* the stream.
*/
-static ulg __initdata bb;/* bit buffer */
-static unsigned __initdata bk; /* bits in bit buffer */
-
static const ush mask_bits[] = {
0x,
0x0001, 0x0003, 0x0007, 0x000f, 0x001f, 0x003f, 0x007f, 0x00ff,
@@ -553,8 +550,8 @@ static int __init inflate_codes(
/* make local copies of globals */
-b = bb; /* initialize bit buffer */
-k = bk;
+b = s->bb;/* initialize bit buffer */
+k = s->bk;
w = s->wp;/* initialize window position */
/* inflate the coded data */
@@ -636,8 +633,8 @@ static int __init inflate_codes(
/* restore the globals from the locals */
s->wp = w;/* restore global window pointer */
-bb = b; /* restore global bit buffer */
-bk = k;
+s->bb = b;/* restore global bit buffer */
+s->bk = k;
/* done */
return 0;
@@ -654,8 +651,8 @@ static int __init inflate_stored(struct gunzip_state *s)
DEBG("bb;/* initialize bit buffer */
+k = s->bk;
w = s->wp;/* initialize window position */
@@ -689,8 +686,8 @@ static int __init inflate_stored(struct gunzip_state *s)
/* restore the globals from the locals */
s->wp = w;/* restore global window pointer */
-bb = b; /* restore global bit buffer */
-bk = k;
+s->bb = b;/* restore global bit buffer */
+s->bk = k;
DEBG(">");
return 0;
@@ -794,8 +791,8 @@ static int noinline __init inflate_dynamic(struct
gunzip_state *s)
return 1;
/* make local bit buffer */
-b = bb;
-k = bk;
+b = s->bb;
+k = s->bk;
/* read in table lengths */
NEEDBITS(s, 5);
@@ -899,8 +896,8 @@ static int noinline __init inflate_dynamic(struct
gunzip_state *s)
DEBG("dyn5 ");
/* restore the global bit buffer */
-bb = b;
-bk = k;
+s->bb = b;
+s->bk = k;
DEBG("dyn5a ");
@@ -965,8 +962,8 @@ static int __init inflate_block(struct gunzip_state *s, int
*e)
DEBG("bb;
+k = s->bk;
/* read in last block bit */
NEEDBITS(s, 1);
@@ -979,8 +976,8 @@ static int __init inflate_block(struct gunzip_state *s, int
*e)
DUMPBITS(2);
/* restore the global bit buffer */
-bb = b;
-bk = k;
+s->bb = b;
+s->bk = k;
/* inflate that block type */
if (t == 2)
@@ -1004,8 +1001,8 @@ static int __init inflate(struct gunzip_state *s)
/* initialize window, bit buffer */
s->wp = 0;
-bk = 0;
-bb = 0;
+s->bk = 0;
+s->bb = 0;
/* decompress until the last block */
do {
@@ -1017,8 +1014,8 @@ static int __init inflate(struct gunzip_state *s)
/* Undo too much lookahead. The next read will be byte aligned so we
* can discard unused bits in the last meaningful byte.
*/
-while (bk >= 8) {
-bk -= 8;
+while (s->bk >= 8) {
+s->bk -= 8;
s->inptr--;
}
--
2.30.2
