From mboxrd@z Thu Jan 1 00:00:00 1970 From: laijs@cn.fujitsu.com (Lai Jiangshan) Date: Mon, 19 Jan 2009 10:51:45 +0800 Subject: [ltt-dev] [PATCH 06/13] introduce ltt_serialize_printf() In-Reply-To: <20090115190829.GF32620@Krystal> References: <496C5A37.2030809@cn.fujitsu.com> <20090115190829.GF32620@Krystal> Message-ID: <4973EAC1.1070204@cn.fujitsu.com> 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 >> --- >> 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 >> >