From mboxrd@z Thu Jan  1 00:00:00 1970
From: laijs@cn.fujitsu.com (Lai Jiangshan)
Date: Mon, 19 Jan 2009 10:27:19 +0800
Subject: [ltt-dev] [PATCH 05/13] read basic type from ltt-relay buffer
In-Reply-To: <20090117043702.GA17265@Krystal>
References: <496C5A2D.2040300@cn.fujitsu.com> <20090115184859.GE32620@Krystal>
	<49714F54.6080009@cn.fujitsu.com> <20090117043702.GA17265@Krystal>
Message-ID: <4973E507.4010009@cn.fujitsu.com>

Mathieu Desnoyers wrote:
> * Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
>> 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?
>>
> 
> Not necessarily 2 functions. Just separating it in two switch () statements
> would do the job.
> 

I don't understand what you mean.
My code is the smallest, if we don't use macro.

> Mathieu
> 
>>> 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
>>>
>>>
>>
>> _______________________________________________
>> ltt-dev mailing list
>> ltt-dev at lists.casi.polymtl.ca
>> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
>>
>