On Thu, Oct 21, 2021 at 04:55:19PM +0300, Nikolay Borisov wrote:
>
>
> On 1.09.21 г. 20:01, Omar Sandoval wrote:
> > From: Boris Burkov <boris@xxxxxx>
> >
> > +static int decompress_lzo(const char *encoded_data, u64 encoded_len,
> > + char *unencoded_data, u64 unencoded_len,
> > + unsigned int page_size)
> > +{
> > + uint32_t total_len;
> > + size_t in_pos, out_pos;
> > +
> > + if (encoded_len < 4) {
> > + error("lzo header is truncated");
> > + return -EIO;
> > + }
> > + memcpy(&total_len, encoded_data, 4);
> > + total_len = le32toh(total_len);
> > + if (total_len > encoded_len) {
> > + error("lzo header is invalid");
> > + return -EIO;
> > + }
> > +
> > + in_pos = 4;
> > + out_pos = 0;
> > + while (in_pos < total_len && out_pos < unencoded_len) {
> > + size_t page_remaining;
> > + uint32_t src_len;
> > + lzo_uint dst_len;
> > + int ret;
> > +
> > + page_remaining = -in_pos % page_size;
>
> Why the -in_pos?
in_pos is our position in the encoded data. This calculates how many
bytes are remaining in the page that in_pos current points to.
> > + if (page_remaining < 4) {
> > + if (total_len - in_pos <= page_remaining)
> > + break;
> > + in_pos += page_remaining;
> > + }
> > +
> > + if (total_len - in_pos < 4) {
> > + error("lzo segment header is truncated");
> > + return -EIO;
> > + }
> > +
> > + memcpy(&src_len, encoded_data + in_pos, 4);
> > + src_len = le32toh(src_len);
> > + in_pos += 4;
> > + if (src_len > total_len - in_pos) {
> > + error("lzo segment header is invalid");
> > + return -EIO;
> > + }
> > +
> > + dst_len = page_size;
> > + ret = lzo1x_decompress_safe((void *)(encoded_data + in_pos),
> > + src_len,
> > + (void *)(unencoded_data + out_pos),
> > + &dst_len, NULL);
> > + if (ret != LZO_E_OK) {
> > + error("lzo1x_decompress_safe failed: %d", ret);
> > + return -EIO;
> > + }
> > +
> > + in_pos += src_len;
> > + out_pos += dst_len;
> > + }
> > + return 0;
> > +}
> > +
> > +static int decompress_and_write(struct btrfs_receive *rctx,
> > + const char *encoded_data, u64 offset,
> > + u64 encoded_len, u64 unencoded_file_len,
> > + u64 unencoded_len, u64 unencoded_offset,
> > + u32 compression)
> > +{
> > + int ret = 0;
> > + size_t pos;
> > + ssize_t w;
> > + char *unencoded_data;
> > + int page_shift;
> > +
> > + unencoded_data = calloc(unencoded_len, 1);
> > + if (!unencoded_data) {
> > + error("allocating space for unencoded data failed: %m");
> > + return -errno;
> > + }
> > +
> > + switch (compression) {
> > + case BTRFS_ENCODED_IO_COMPRESSION_ZLIB:
> > + ret = decompress_zlib(rctx, encoded_data, encoded_len,
> > + unencoded_data, unencoded_len);
> > + if (ret)
> > + goto out;
> > + break;
> > + case BTRFS_ENCODED_IO_COMPRESSION_ZSTD:
> > + ret = decompress_zstd(rctx, encoded_data, encoded_len,
> > + unencoded_data, unencoded_len);
> > + if (ret)
> > + goto out;
> > + break;
> > + case BTRFS_ENCODED_IO_COMPRESSION_LZO_4K:
> > + case BTRFS_ENCODED_IO_COMPRESSION_LZO_8K:
> > + case BTRFS_ENCODED_IO_COMPRESSION_LZO_16K:
> > + case BTRFS_ENCODED_IO_COMPRESSION_LZO_32K:
> > + case BTRFS_ENCODED_IO_COMPRESSION_LZO_64K:
> > + page_shift = compression - BTRFS_ENCODED_IO_COMPRESSION_LZO_4K + 12;
>
> Doesn't this calculation assume page size is 4k, what about arches with
> larger page size (ppc/aarch64), shouldn't that '12' be adjusted?
This is unrelated to the machine page size. It is translating the
BTRFS_ENCODED_IO_COMPRESSION_LZO_* value to the page size used for
compressing the data:
compression | - LZO_4K | + 12 | 1 <<
=====================================
LZO_4K = 3 | 0 | 12 | 4096
LZO_8K = 4 | 1 | 13 | 8192
LZO_16K = 5 | 2 | 14 | 16384
LZO_32K = 6 | 3 | 15 | 32768
LZO_64K = 7 | 4 | 16 | 65536
I'll fix the other comments, thanks.
