Re: [PATCH] trace-cmd: fix writing of uncompressed size

[Sven Schnelle <svens@linux.ibm.com>]

Tzvetomir Stoyanov <tz.stoyanov@gmail.com> writes:

> On Mon, Jul 11, 2022 at 10:44 AM Sven Schnelle <svens@linux.ibm.com> wrote:
>>
>> Pass &size instead of &handle->pointer. Interestingly this doesn't hurt
>> on x86, but makes trace-cmd fail on s390.
>
> Hi Sven,
> Thanks for testing this code on s390, I've tested it only on x86.
> Please, can you give more information about the trace-cmd failure on
> s390?
>
>>
>> Fixes: 3f8447b1 ("trace-cmd library: Add support for compression algorithms")
>> Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
>> ---
>>  lib/trace-cmd/trace-compress.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/trace-cmd/trace-compress.c b/lib/trace-cmd/trace-compress.c
>> index a63295e..ad9b7fc 100644
>> --- a/lib/trace-cmd/trace-compress.c
>> +++ b/lib/trace-cmd/trace-compress.c
>> @@ -331,7 +331,7 @@ int tracecmd_compress_block(struct tracecmd_compression *handle)
>>                 goto out;
>>
>>         /* Write uncompressed data size */
>> -       endian4 = tep_read_number(handle->tep, &handle->pointer, 4);
>> +       endian4 = tep_read_number(handle->tep, &size, 4);
>
> Here 'size' is the size of the buffer, used by the compression
> algorithm to compress the data block. That size depends on the
> algorithm, but usually it is less than the uncompressed data size and
> bigger than the compressed data size. On this position in the file
> must be written the size of the uncompressed data, that is
> 'handle->pointer'. I agree that the name is a bit misleading, as this
> is not a pointer to a memory address.  The 'handle->pointer' is the
> offset within the compression buffer, where the next uncompressed data
> will be written. The logic uses a dynamic buffer with given capacity.
> When the buffer is empty, 'handle->pointer' is 0. With each
> uncompressed data chunk, written into that buffer, 'handle->pointer'
> increases with its size - i.e. the first byte is written on position 0
> (the initial 'handle->pointer') and the pointer increases to 1. That's
> why it reflects the size of the uncompressed data. When the pointer
> reaches the capacity of the buffer, the buffer is extended. At the
> end, that buffer is passed to the compression algorithm, to compress
> the block.

I see that 'handle->pointer' is unsigned long, which is 8 bytes on
s390. But it is converted as 4 byte int. That would work on LE
platforms, but not on BE.