[ltt-dev] [PATCH 05/13] read basic type from ltt-relay buffer

[laijs@cn.fujitsu.com (Lai Jiangshan)]

Mathieu Desnoyers wrote:
> * Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
>> we need function read the base type from ltt-relay buffer
>> in the kernel for text output
>>
>> Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
>> ---
>>  ltt-serialize.c |   71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 71 insertions(+)
>> diff --git a/ltt/ltt-serialize.c b/ltt/ltt-serialize.c
>> index 5c34899..deb9106 100644
>> --- a/ltt/ltt-serialize.c
>> +++ b/ltt/ltt-serialize.c
>> @@ -531,6 +538,77 @@
>>  }
>>  EXPORT_SYMBOL_GPL(ltt_serialize_data);
>>  
>> +static inline u64 serialize_read_base_type(struct rchan_buf *buf, size_t *ppos,
> 
> Maybe the name "unserialize" would be more appropriate ?
> 
>> +		char trace_size, enum ltt_type trace_type)
>> +{
>> +	union {
>> +		uint8_t  u_8;
>> +		uint16_t u_16;
>> +		uint32_t u_32;
>> +		uint64_t u_64;
>> +		int8_t   s_8;
>> +		int16_t  s_16;
>> +		int32_t  s_32;
>> +		int64_t  s_64;
>> +	} tmp;
>> +	uint64_t value;
>> +
>> +	if (ltt_get_alignment())
> 
> This if () is unneeded. It's already in the ltt_align logic.
> 
>> +		*ppos += ltt_align(*ppos, trace_size);
>> +
>> +	switch (trace_type) {
>> +	case LTT_TYPE_SIGNED_INT:
>> +		switch (trace_size) {
>> +		case 1:
>> +			ltt_relay_read(buf, *ppos, &tmp.s_8, trace_size);
>> +			value = tmp.s_8;
> 
> I think we could separate that in two parts to make the code smaller :

separate it into 2 functions?

> 
> 1 - 
> ltt_relay_read for u8, u16, u32, u64 (not caring about singedness)
> 
> 2 - depending on the sign, cast to (s8, s16, s32) and then cast to a
> 64-bit signed value. Or if it was already 64-bits, no conversion is
> needed.
> 
> The end result (64-bit signed or unsigned value) should in the end be
> casted to a u64.
> 
> Mathieu
> 
>