[ltt-dev] [PATCH 06/13] introduce ltt_serialize_printf()

[Mathieu Desnoyers]

* Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
> introduce ltt_serialize_printf() for format binary data which is in
> ltt-relay buffer to human readable text string.
> 
> Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
> ---
>  b/ltt/ltt-serialize.c      |  108 ++++++++++++++++++++++++++++++++++++++++++++-
>  include/linux/ltt-tracer.h |    3 +
>  2 files changed, 109 insertions(+), 2 deletions(-)
> diff --git a/include/linux/ltt-tracer.h b/include/linux/ltt-tracer.h
> index ece6819..b49cac2 100644
> --- a/include/linux/ltt-tracer.h
> +++ b/include/linux/ltt-tracer.h
> @@ -118,6 +119,9 @@ extern void ltt_vtrace(const struct marker *mdata, void *probe_data,
>  extern void ltt_trace(const struct marker *mdata, void *probe_data,
>  	void *call_data, const char *fmt, ...);
>  
> +size_t ltt_serialize_printf(struct rchan_buf *buf, size_t buf_offset,
> +		u32 *msg_size, char *output, size_t outlen, const char *fmt);
> +
>  /*
>   * Unique ID assigned to each registered probe.
>   */
> 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
> @@ -227,7 +227,7 @@ parse_end:
>   * %n not supported.
>   */
>  static inline const char *parse_c_type(const char *fmt,
> -		char *c_size, enum ltt_type *c_type)
> +		char *c_size, enum ltt_type *c_type, char *outfmt)
>  {
>  	int qualifier;		/* 'h', 'l', or 'L' for integer fields */
>  				/* 'z' support added 23/7/1999 S.H.    */
> @@ -259,6 +259,13 @@ repeat:
>  		}
>  	}
>  
> +	if (outfmt) {
> +		if (qualifier != -1)
> +			*outfmt++ = (char)qualifier;
> +		*outfmt++ = *fmt;
> +		*outfmt = 0;
> +	}
> +
>  	switch (*fmt) {
>  	case 'c':
>  		*c_type = LTT_TYPE_UNSIGNED_INT;
> @@ -502,7 +509,7 @@ notrace size_t ltt_serialize_data(struct rchan_buf *buf, size_t buf_offset,
>  			++fmt;			/* skip first '%' */
>  			if (*fmt == '%')	/* Escaped %% */
>  				break;
> -			fmt = parse_c_type(fmt, &c_size, &c_type);
> +			fmt = parse_c_type(fmt, &c_size, &c_type, NULL);
>  			/*
>  			 * Output c types if no trace types has been
>  			 * specified.
> @@ -531,6 +538,103 @@
>  	return value;
>  }
>  
> +static inline int serialize_printf_data(struct rchan_buf *buf, size_t *ppos,
> +		char trace_size, enum ltt_type trace_type,
> +		char c_size, enum ltt_type c_type,
> +		char *output, size_t outlen, const char *outfmt)
> +{
> +	u64 value;
> +	outlen = (ssize_t)outlen < 0 ? 0 : outlen;
> +
> +	if (trace_type == LTT_TYPE_STRING) {
> +		size_t len = ltt_relay_read_cstr(buf, *ppos, output, outlen);
> +		*ppos += len + 1;
> +		return len;
> +	}
> +
> +	value = serialize_read_base_type(buf, ppos, trace_size, trace_type);
> +
> +	if (c_size == 8)
> +		return snprintf(output, outlen, outfmt, value);
> +	else
> +		return snprintf(output, outlen, outfmt, (int)value);

Given value is u64, I'd use unsigned int cast. Should not make much
difference in the end because there is no sign to extend, but I prefer
to play safe.

> +}
> +

kerneldocs header would be useful here... what is "output" exactly ? I
can figure it out, but it took me a few seconds to be sure. :)

Does outlen include space for the final \0 ? Or maybe there is no final
\0 at the end of output ... this should be specified.

> +size_t ltt_serialize_printf(struct rchan_buf *buf, size_t buf_offset,
> +		u32 *msg_size, char *output, size_t outlen, const char *fmt)
> +{
> +	char trace_size = 0, c_size = 0;	/*
> +						 * 0 (unset), 1, 2, 4, 8 bytes.
> +						 */
> +	enum ltt_type trace_type = LTT_TYPE_NONE, c_type = LTT_TYPE_NONE;
> +	unsigned long attributes = 0;
> +	char outfmt[4] = "%";
> +	size_t outpos = 0, len;

I find the line above surprising. can we declare len on a separate line?
Or we should concatenate the next msgpos = buf_offset line...

> +	size_t msgpos = buf_offset;
> +	outlen = (ssize_t)outlen < 0 ? 0 : outlen;

would be clearer with
outlen = min_t(ssize_t, 0, outlen);

> +
> +	for (; *fmt ; ++fmt) {
> +		switch (*fmt) {
> +		case '#':
> +			/* tracetypes (#) */
> +			++fmt;			/* skip first '#' */
> +			if (*fmt == '#') {	/* Escaped ## */
> +				if (outpos < outlen)
> +					output[outpos] = '#';
> +				outpos++;
> +				break;
> +			}
> +			attributes = 0;
> +			fmt = parse_trace_type(fmt, &trace_size, &trace_type,
> +				&attributes);
> +			break;
> +		case '%':
> +			/* c types (%) */
> +			++fmt;			/* skip first '%' */
> +			if (*fmt == '%') {	/* Escaped %% */
> +				if (outpos < outlen)
> +					output[outpos] = '%';
> +				outpos++;
> +				break;
> +			}
> +			fmt = parse_c_type(fmt, &c_size, &c_type, outfmt + 1);
> +			/*
> +			 * Output c types if no trace types has been
> +			 * specified.
> +			 */
> +			if (!trace_size)
> +				trace_size = c_size;
> +			if (trace_type == LTT_TYPE_NONE)
> +				trace_type = c_type;
> +			if (c_type == LTT_TYPE_STRING)
> +				trace_type = LTT_TYPE_STRING;
> +
> +			/* perform trace printf */
> +			len = serialize_printf_data(buf, &msgpos, trace_size,
> +					trace_type, c_size, c_type,
> +					output + outpos, outlen - outpos,
> +					outfmt);
> +			outpos += len;
> +			trace_size = 0;
> +			c_size = 0;
> +			trace_type = LTT_TYPE_NONE;
> +			c_size = LTT_TYPE_NONE;
> +			attributes = 0;
> +			break;
> +		default:
> +			if (outpos < outlen)
> +				output[outpos] = *fmt;
> +			outpos++;
> +			break;
> +		}
> +	}

Mathieu

> +	if (msg_size)
> +		*msg_size = msgpos - buf_offset;
> +
> +	return outpos;
> +}
> +EXPORT_SYMBOL_GPL(ltt_serialize_printf);
> +
>  /*
>   * Calculate data size
>   * Assume that the padding for alignment starts at a sizeof(void *) address.
> 
> 
> 
> 
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

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