[Toybox] [PATCH] Clean up xz a good amount

[Rob Landley]

On 2/27/24 21:27, Oliver Webb wrote:
> 2 are help text, do_xzcat was due to replacing the case statement for error handling with a array
> If I had to guess, xz_dec_run or xz_dec_bcj_reset
> 
> It was the case statement I stripped out in xz_dec_bcj_reset, it works with the attached patch

Let's see..

> diff --git a/toys/pending/xzcat.c b/toys/pending/xzcat.c
> index 5177f243..b50c9524 100644
> --- a/toys/pending/xzcat.c
> +++ b/toys/pending/xzcat.c
> @@ -55,6 +55,7 @@ enum xz_ret {
>    // truncated or otherwise corrupt.
>  };
>  
> +
>  // Passing input and output buffers to XZ code
>  // Only the contents of the output buffer from out[out_pos] onward, and
>  // the variables in_pos and out_pos are modified by the XZ code.

Whitespace change.

> @@ -212,7 +213,7 @@ struct xz_dec_bcj {
>  // in an x86 instruction.
>  static int bcj_x86_test_msbyte(char b)
>  {
> -  return !b || !~b;
> +  return !b || b == 0xFF;
>  }

NOP.

>  static size_t bcj_x86(struct xz_dec_bcj *s, char *buf, size_t size)
> @@ -639,6 +640,20 @@ enum xz_ret xz_dec_bcj_run(struct xz_dec_bcj *s, struct xz_dec_lzma2 *lzma2,
>   */
>  enum xz_ret xz_dec_bcj_reset(struct xz_dec_bcj *s, char id)
>  {
> +  switch (id) {
> +  case BCJ_X86:
> +  case BCJ_POWERPC:
> +  case BCJ_IA64:
> +  case BCJ_ARM:
> +  case BCJ_ARMTHUMB:
> +  case BCJ_SPARC:
> +    break;
> +
> +  default:
> +    /* Unsupported Filter ID */
> +    return XZ_OPTIONS_ERROR;
> +  }
> +
>    s->type = id;
>    s->ret = XZ_OK;
>    s->pos = 0;

NOP unless you hit the error path, what are the values...

  enum {
    BCJ_X86 = 4,        /* x86 or x86-64 */
    BCJ_POWERPC = 5,    /* Big endian only */
    BCJ_IA64 = 6,       /* Big or little endian */
    BCJ_ARM = 7,        /* Little endian only */
    BCJ_ARMTHUMB = 8,   /* Little endian only */
    BCJ_SPARC = 9       /* Big or little endian */
  } type;

Since id is char, which is unsigned in toybox, this is:

  if ((id-4)>5) return XZ_OPTIONS_ERROR;

> @@ -2860,16 +2875,17 @@ void do_xzcat(int fd, char *name)
>      if (ret == XZ_OK || ret == XZ_UNSUPPORTED_CHECK)
>        continue;
>  
> -    else if (ret == XZ_STREAM_END) {
> -      xz_dec_end(s);
> -      return;
> -    }
> -
>      if (fwrite(out, 1, b.out_pos, stdout) != b.out_pos) {
>        msg = "Write error\n";
>        goto error;
>      }
>  
> +    if (ret == XZ_STREAM_END) {
> +      xz_dec_end(s);
> +      return;
> +    }
> +
> +
>      msg = (ret-3 < ARRAY_LEN(errors)) ? errors[ret-3] : "Bug!";
>      goto error;
>    }

There's the bug, moving a return before the fwrite().

Rob