[ltt-dev] [PATCH] Add modified formatedDump module: Use glib function for solving buffer overflow problem.
Mathieu Desnoyers
compudj at krystal.dyndns.org
Fri Jan 14 23:33:01 EST 2011
Can you resend as a standalone patch ? It will be easier to review than
a patch based on your previous one.
Thanks,
Mathieu
* Vincent Attard (vincent.attard at polymtl.ca) wrote:
> ---
> lttv/modules/text/formatedDump.c | 133 +++++++++++++++----------------------
> 1 files changed, 54 insertions(+), 79 deletions(-)
>
> diff --git a/lttv/modules/text/formatedDump.c b/lttv/modules/text/formatedDump.c
> index 44f6589..58ccdb8 100644
> --- a/lttv/modules/text/formatedDump.c
> +++ b/lttv/modules/text/formatedDump.c
> @@ -14,7 +14,7 @@
> *
> * You should have received a copy of the GNU General Public License
> * along with this program; if not, write to the Free Software
> - * Foundation, Inc., 59 Temple Place - Suite 330, Boston,
> + * Foundation, Inc., 59 Temple Place - Suite 330, Boston,
> * MA 02111-1307, USA.
> */
>
> @@ -44,25 +44,20 @@
> #include <string.h>
> #include <stdlib.h>
>
> -static gboolean
> -a_noevent,
> -a_no_field_names,
> -a_state,
> -a_text;
> +static gboolean a_noevent;
> +static gboolean a_no_field_names;
> +static gboolean a_state;
> +static gboolean a_text;
>
> -static char
> -*a_file_name = NULL;
> -static char
> -*a_format = NULL;
> +static char *a_file_name = NULL;
> +static char *a_format = NULL;
>
> -static LttvHooks
> -*before_traceset,
> -*event_hook;
> +static LttvHooks *before_traceset;
> +static LttvHooks *event_hook;
>
> static char default_format[] =
> "tracefile:%f envent:%e timestamp:%t elapse:%l cpu:%c pid:%d tgpid:%g "\
> "process:%p brand:%b ppid:%i state:%a";
> -
> static char textDump_format[] =
> "%f.%e: %s.%n (%r/%f_%c), %d, %g, %p, %b, %i, %y, %a { %m }";
>
> @@ -74,10 +69,10 @@ static gboolean open_output_file(void *hook_data, void *call_data)
> {
> g_info("Open the output file");
>
> - if(a_file_name == NULL) a_file = stdout;
> + if (a_file_name == NULL) a_file = stdout;
> else a_file = fopen(a_file_name, "w");
>
> - if(a_file == NULL) g_error("cannot open file %s", a_file_name);
> + if (a_file == NULL) g_error("cannot open file %s", a_file_name);
> return FALSE;
> }
>
> @@ -114,14 +109,14 @@ static int write_event_content(void *hook_data, void *call_data)
> /*
> * call to the filter if available
> */
> - if(filter->head != NULL)
> - if(!lttv_filter_tree_parse(filter->head,e,tfc->tf,
> + if (filter->head != NULL)
> + if (!lttv_filter_tree_parse(filter->head,e,tfc->tf,
> tfc->t_context->t,tfc,NULL,NULL))
> return FALSE;
>
> lttv_event_to_string(e, a_string, TRUE, !a_no_field_names, tfs);
>
> - if(a_state) {
> + if (a_state) {
> g_string_append_printf(a_string, "%s ",
> g_quark_to_string(process->state->s));
> }
> @@ -133,7 +128,7 @@ static int write_event_content(void *hook_data, void *call_data)
> return FALSE;
> }
>
> -void lttv_event_to_string(LttEvent *e, GString *s, gboolean mandatory_fields,
> +void lttv_event_to_string(LttEvent *e, GString *string_buffer, gboolean mandatory_fields,
> gboolean field_names, LttvTracefileState *tfs)
> {
> struct marker_field *field;
> @@ -147,13 +142,11 @@ void lttv_event_to_string(LttEvent *e, GString *s, gboolean mandatory_fields,
> LttvTraceState *ts = (LttvTraceState*)tfs->parent.t_context;
> LttvProcessState *process = ts->running_process[cpu];
>
> - s = g_string_set_size(s,0);
> -
> info = marker_get_info_from_id(tfs->parent.tf->mdata, e->event_id);
> - if(mandatory_fields) {
> + if (mandatory_fields) {
> time = ltt_event_time(e);
> - /* Calculate elapse between current and previous event time */
> - if (time_prev.tv_sec==0 && time_prev.tv_nsec == 0) {
> + /* Calculate elapsed time between current and previous event */
> + if (time_prev.tv_sec == 0 && time_prev.tv_nsec == 0) {
> time_prev = ltt_event_time(e);
> elapse.tv_sec = 0;
> elapse.tv_nsec = 0;
> @@ -165,113 +158,97 @@ void lttv_event_to_string(LttEvent *e, GString *s, gboolean mandatory_fields,
>
> char * fmt;
> int i;
> - int fmt_len = 0;
> - if(a_text) {
> +
> + if (a_text) {
> /* textDump format (used with -T command option) */
> fmt = textDump_format;
> - fmt_len = strlen(textDump_format);
> }
>
> else if (!a_format) {
> /* Default format (used if no option) */
> fmt = default_format;
> - fmt_len = strlen(default_format);
> } else {
> /* formatedDump format
> * (used with -F command option following by the desired format) */
> fmt = a_format;
> - fmt_len = strlen(a_format);
> }
>
> - char str[1024] = {0};
> + g_string_set_size(string_buffer, 0);
> /* Switch case:
> - * all '%-' are replaced by the desired value in 'str' */
> - char *str_pos = str;
> - for (i = 0; i < strlen(fmt); i++){
> - if (fmt[i] == '%'){
> -
> - switch (fmt[++i]){
> + * all '%-' are replaced by the desired value in 'string_buffer' */
> + for (i = 0; i < strlen(fmt); i++) {
> + if (fmt[i] == '%') {
> + switch (fmt[++i]) {
> case 't':
> - str_pos += sprintf(str_pos, "%ldh%02ldm%02lds%09ldns",
> + g_string_append_printf(string_buffer, "%ldh%02ldm%02lds%09ldns",
> time.tv_sec/3600, (time.tv_sec%3600)/60, time.tv_sec%60,
> time.tv_nsec);
> break;
> case 'f':
> - strcpy(str_pos,
> + g_string_append(string_buffer,
> g_quark_to_string(ltt_tracefile_name(tfs->parent.tf)));
> - str_pos = str_pos + strlen(g_quark_to_string(ltt_tracefile_name(tfs->parent.tf)));
> break;
> case 'e':
> - strcpy(str_pos,
> + g_string_append(string_buffer,
> g_quark_to_string(info->name));
> - str_pos = str_pos + strlen(g_quark_to_string(info->name));
> break;
> case 'd':
> - str_pos += sprintf(str_pos, "%u", process->pid);
> + g_string_append_printf(string_buffer, "%u", process->pid);
> break;
> case 's':
> - str_pos += sprintf(str_pos, "%lu", time.tv_sec);
> + g_string_append_printf(string_buffer, "%ld", time.tv_sec);
> break;
> case 'n':
> - str_pos += sprintf(str_pos, "%lu", time.tv_nsec);
> + g_string_append_printf(string_buffer, "%ld", time.tv_nsec);
> break;
> case 'i':
> - str_pos += sprintf(str_pos, "%u", process->ppid);
> + g_string_append_printf(string_buffer, "%u", process->ppid);
> break;
> case 'g':
> - str_pos += sprintf(str_pos, "%u", process->tgid);
> + g_string_append_printf(string_buffer, "%u", process->tgid);
> break;
> case 'p':
> - strcpy(str_pos,g_quark_to_string(process->name));
> - str_pos = str_pos + strlen(g_quark_to_string(process->name));
> + g_string_append(string_buffer,
> + g_quark_to_string(process->name));
> break;
> case 'b':
> - strcpy(str_pos,g_quark_to_string(process->brand));
> - str_pos = str_pos + strlen(g_quark_to_string(process->brand));
> + g_string_append(string_buffer,
> + g_quark_to_string(process->brand));
> break;
> case 'c':
> - str_pos += sprintf(str_pos,"%u", cpu);
> + g_string_append_printf(string_buffer, "%u", cpu);
> break;
> case 'l':
> - str_pos += sprintf(str_pos, "%lds%09ldns",
> + g_string_append_printf(string_buffer, "%lds%09ldns",
> elapse.tv_sec, elapse.tv_nsec);
> break;
> case 'a':
> - strcpy(str_pos,g_quark_to_string(process->state->t));
> - str_pos = str_pos + strlen(g_quark_to_string(process->state->t));
> + g_string_append(string_buffer,
> + g_quark_to_string(process->state->t));
> break;
> case 'm':
> {
> - GString *marker = g_string_new(0);
> -
> /* Get and print markers and tracepoints fields into 'marker' */
> - if(marker_get_num_fields(info) == 0) return;
> + if (marker_get_num_fields(info) == 0) break;
> for (field = marker_get_field(info, 0);
> field != marker_get_field(info, marker_get_num_fields(info));
> field++) {
> - if(field != marker_get_field(info, 0)) {
> - g_string_append_printf(marker, ", ");
> + if (field != marker_get_field(info, 0)) {
> + g_string_append(string_buffer, ", ");
> }
> - lttv_print_field(e, field, marker, field_names, tfs);
> -
> + lttv_print_field(e, field, string_buffer, field_names, tfs);
> }
> - strcpy(str_pos,marker->str);
> - str_pos = str_pos + marker->len;
> - g_string_free(marker,0);
> }
> break;
> case 'r':
> - strcpy(str_pos,g_quark_to_string(ltt_trace_name
> - (ltt_tracefile_get_trace(tfs->parent.tf))));
> - str_pos = str_pos + strlen(g_quark_to_string(ltt_trace_name
> - (ltt_tracefile_get_trace(tfs->parent.tf))));
> + g_string_append(string_buffer, g_quark_to_string(
> + ltt_trace_name(ltt_tracefile_get_trace(tfs->parent.tf))));
> break;
> case '%':
> - *str_pos = '%';
> - str_pos = str_pos + 1;
> + g_string_append_c(string_buffer, '%');
> break;
> case 'y':
> - str_pos += sprintf(str_pos, "0x%" PRIx64,
> + g_string_append_printf(string_buffer, "0x%" PRIx64,
> process->current_function);
> break;
>
> @@ -280,12 +257,9 @@ void lttv_event_to_string(LttEvent *e, GString *s, gboolean mandatory_fields,
> else
> {
> /* Copy every character if not equals to '%' */
> - *str_pos = fmt[i];
> - str_pos++;
> + g_string_append_c(string_buffer, fmt[i]);
> }
> }
> - *str_pos = '\0';
> - g_string_append_printf(s, "%s", str);
> }
>
> static void init()
> @@ -311,10 +285,10 @@ static void init()
> "output the desired format\n\
> FORMAT controls the output. Interpreted sequences are:\n\
> \n\
> - %f tracefile name\n\
> - %r path to trace\n\
> - %e event name\n\
> + %f channel name\n\
> %p process name\n\
> + %e event name\n\
> + %r path to trace\n\
> %t timestamp (e.g., 2h08m54s025684145ns)\n\
> %s seconds\n\
> %n nanoseconds\n\
> @@ -325,6 +299,7 @@ static void init()
> %c cpu\n\
> %b brand\n\
> %a state\n\
> + %y memory address\n\
> %m markers and tracepoints fields\n",
> "format string",
> LTTV_OPT_STRING, &a_format, NULL, NULL);
> --
> 1.7.0.4
>
>
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
>
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
More information about the lttng-dev
mailing list