From mboxrd@z Thu Jan  1 00:00:00 1970
From: compudj@krystal.dyndns.org (Mathieu Desnoyers)
Date: Mon, 19 Jan 2009 20:45:31 -0500
Subject: [ltt-dev] [PATCH 05/13] read basic type from ltt-relay buffer
In-Reply-To: <4973E507.4010009@cn.fujitsu.com>
References: <496C5A2D.2040300@cn.fujitsu.com> <20090115184859.GE32620@Krystal>
	<49714F54.6080009@cn.fujitsu.com> <20090117043702.GA17265@Krystal>
	<4973E507.4010009@cn.fujitsu.com>
Message-ID: <20090120014531.GB9224@Krystal>

* Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
> 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.
> >>>

Here is a modified version. It may not be the smallest cost in terms of
LOC, but what I am trying to do here is to only have 4 "ltt_relay_read"
function calls, because all ltt_relay_read has to know is the size to
read, it does not care about types. Type management is made below in the
2nd switch() statement.


static inline u64 serialize_read_base_type(struct rchan_buf *buf, size_t *ppos,
		char trace_size, enum ltt_type trace_type)
{
	union {
		u8  u_8;
		u16 u_16;
		u32 u_32;
		u64 u_64;
	} tmp;
	uint64_t value;

	if (ltt_get_alignment())
		*ppos += ltt_align(*ppos, trace_size);

	switch (trace_size) {
	case 1:
		ltt_relay_read(buf, *ppos, &tmp.u_8, trace_size);
		break;
	case 2:
		ltt_relay_read(buf, *ppos, &tmp.u_16, trace_size);
		break;
	case 4:
		ltt_relay_read(buf, *ppos, &tmp.u_32, trace_size);
		break;
	case 8:
		ltt_relay_read(buf, *ppos, &tmp.u_64, trace_size);
		break;
	default:
		BUG();
	}

	switch (trace_type) {
	case LTT_TYPE_SIGNED_INT:
		switch (trace_size) {
		case 1:
			value = (s64)(s8)tmp.u_8;
			break;
		case 2:
			value = (s64)(s16)tmp.u_16;
			break;
		case 4:
			value = (s64)(s32)tmp.u_32;
			break;
		case 8:
			value = tmp.u_64;
			break;
		default:
			BUG();
		}
		break;
	case LTT_TYPE_UNSIGNED_INT:
		switch (trace_size) {
		case 1:
			value = (u64)tmp.u_8;
			break;
		case 2:
			value = (u64)tmp.u_16;
			break;
		case 4:
			value = (u64)tmp.u_32;
			break;
		case 8:
			value = tmp.u_64;
			break;
		default:
			BUG();
		}
		break;
	default:
		BUG();
	}

	*ppos += trace_size;
	return value;
}

 /*
  * Calculate data size
  * Assume that the padding for alignment starts at a sizeof(void *) address.

Mathieu

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68