[ltt-dev] [PATCH 06/13] introduce ltt_serialize_printf()
Lai Jiangshan
laijs at cn.fujitsu.com
Sun Jan 18 21:51:45 EST 2009
Mathieu Desnoyers wrote:
> * 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.
There are the same.
>
>> +}
>> +
>
> 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...
How surprise comes from? how about I rename len to outedlen?
len is used for output buffer, I think "size_t outpos = 0, len;" is good.
msgpos is for relay buffer.
>
>> + 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
>>
>
More information about the lttng-dev
mailing list