From e8548639b44b22ef7db0757563dd58a741a322cd Mon Sep 17 00:00:00 2001
From: pmf <pmf@04897980-b3bd-0310-b5e0-8ef037075253>
Date: Wed, 6 Jun 2007 02:29:22 +0000
Subject: [PATCH] apply patch by Gabriel Matni to add format string support for
 event fields

git-svn-id: http://ltt.polymtl.ca/svn@2534 04897980-b3bd-0310-b5e0-8ef037075253
---
 genevent/parser.c                   |   8 +-
 ltt/branches/poly/ltt/facility.c    | 383 +++++++++++++++++++++++++++-
 ltt/branches/poly/ltt/ltt-private.h |   4 +
 ltt/branches/poly/ltt/parser.c      |   8 +-
 ltt/branches/poly/ltt/type.c        |   3 +
 ltt/branches/poly/lttv/lttv/print.c |  80 +++---
 6 files changed, 450 insertions(+), 36 deletions(-)

diff --git a/genevent/parser.c b/genevent/parser.c
index 0b725ecc..7ac11e7e 100644
--- a/genevent/parser.c
+++ b/genevent/parser.c
@@ -1,3 +1,6 @@
+
+
+
 /*
 
 parser.c: Generate helper declarations and functions to trace events
@@ -215,6 +218,7 @@ void getTypeAttributes(parse_file_t *in, type_descriptor_t *t,
     if(!strcmp("format",token)) {
       getEqual(in);
       t->fmt = allocAndCopy(getQuotedString(in));
+      // printf("%s - ",t->fmt);
     //} else if(!strcmp("name",token)) {
      // getEqual(in);
      // car = seekNextChar(in);
@@ -1474,7 +1478,9 @@ unsigned long getTypeChecksum(unsigned long aCrc, type_descriptor_t * type)
   crc = partial_crc32(str,crc);
   if(flag) free(str);
 
-  if(type->fmt) crc = partial_crc32(type->fmt,crc);
+  //the format string is not included in the crc calculation
+
+  //if(type->fmt) crc = partial_crc32(type->fmt,crc);
 
   if(type->type == ARRAY){
     crc = getTypeChecksum(crc,((field_t*)type->fields.array[0])->type);
diff --git a/ltt/branches/poly/ltt/facility.c b/ltt/branches/poly/ltt/facility.c
index 380689b4..65c86893 100644
--- a/ltt/branches/poly/ltt/facility.c
+++ b/ltt/branches/poly/ltt/facility.c
@@ -1,3 +1,4 @@
+
 /* This file is part of the Linux Trace Toolkit viewer
  * Copyright (C) 2003-2004 Xiangxiu Yang
  *               2005 Mathieu Desnoyers
@@ -63,6 +64,200 @@ void freeLttField(LttField * fld);
 void freeLttNamedType(LttType * type);
 
 
+/*****************************************************************************
+ *Function name
+ *    parse_fmt               : parses the format string
+ *Input params
+ *    fmt                     : format string specified in the xml file
+ *    header                  : points to the extracted header string
+ *    separator               : points to the extracted separator string
+ *    footer                  : points to the extracted footer string
+ *
+ *    Parses the format string in order to extract the header,
+ *    the separator and the footer.
+ *    The default fmt string is: { %S, %S }
+ *    In this case:
+ *    The header is: {
+ *    The separator is: ,
+ *    The footer
+ *
+ ****************************************************************************/
+
+parse_fmt(char *fmt, char **header, char **separator, char **footer){
+  int i;
+  unsigned int num = 0;
+
+  int header_index = 0;//header index
+  int separator_index = 0;//separator index
+  int footer_index = 0;//footer index
+  int fmt_length = strlen(fmt);
+
+  for(i=0; i<fmt_length; i++)
+    {
+      if(fmt[i]=='%')
+	{
+	  num++;
+	  if(num>=3)
+	    g_error("More than 2 '%%' chars were encountered: %s\n",fmt);
+
+	  //Detecting identifier
+	  if(fmt[++i]!='S')
+	    g_error("Unexpected format: %s\n",fmt);
+
+	  if(i+1<strlen(fmt))
+	    i = i+1;
+	  else
+	    g_error("Unexpected format: %s\n",fmt);
+
+	  //Need to have at least on char after the %S
+	  if(fmt[i]=='%')
+	    g_error("Missing separator: %s\n",fmt);
+
+	  if(separator_index==0)
+	    separator_index = i;//index of separator inside the original string.
+	  else if(footer_index==0)//the 'else' gives the priority to set the separator index first.
+	    footer_index = i;//no break since an error should be generated if more than 2 '%' were encountered
+	}
+    }
+
+
+  //Create header String
+  num =  separator_index-header_index-2;//(-2 due to '%S')
+  if(num<0)
+    g_error("Unexpected format");
+  //*header = malloc(num+1);//(+1 for EOS).
+  *header = g_new(gchar,num+1);//(+1 for EOS).
+  strncpy(*header,fmt,num);
+  (*header)[num] = '\0';//need EOS, not handled by strncopy
+
+  //Create seperator String
+  num =footer_index - separator_index - 2;
+  if(num<0)
+    g_error("Unexpected format");
+  //*separator = malloc(num+1);//+1 for EOS
+  *separator = g_new(gchar, num+1);//+1 for EOS
+  strncpy(*separator,fmt+separator_index*sizeof(char),num);
+  (*separator)[num] = '\0';//need EOS, not handled by strncopy
+
+  //Create footer String
+  num = strlen(fmt)-footer_index;
+  //*footer = malloc(num+1);
+  *footer = g_new(gchar, num+1);
+  strncpy(*footer,fmt+footer_index*sizeof(char),num);
+  (*footer)[num] = '\0';
+}
+
+/*****************************************************************************
+ *Function name
+ *    verify_fmt_syntax       : verifies the syntax of the format string
+ *Input params
+ *    fmt                     : format string specified in the xml file
+ *    fmt_type                : points to the detected type in the fmt string
+ *
+ *    Verifies the syntax of the format string sepcified in the xml file
+ *    It allows fmt strings having the following syntax:
+ *    "..header text... %12.20d ...footer text..."
+ *    It sets the fmt_type accordingly. In the previous example
+ *    fmt_type would point to the character 'd' inside fmt.
+ *
+ *returns 1 on success, and -1 or -2 on error.
+ *
+ ****************************************************************************/
+
+int verify_fmt_syntax(char *fmt, char **fmt_type){
+  int i;
+  int dot = 0;//number of dots encountered.
+  unsigned short counter = 0;
+  *fmt_type=NULL;
+
+
+  for(i=0; i<strlen(fmt); i++)
+    {
+      if(fmt[i]=='%')
+	{
+	  if(++counter==2)
+	    return -1;//should generate an error.
+
+	  i = i+1;//go to next character
+
+	  if(fmt[i]=='-' && i<(strlen(fmt)-1))
+	    i++;//allow this character after the '%'
+
+	  if(fmt[i]=='#' && i<(strlen(fmt)-1))
+	    i++;//allow this character
+	}
+
+      if(counter ==1 && !isdigit(fmt[i]) && *fmt_type==NULL)
+	     switch (fmt[i]){
+	     case 'd':
+	     case 'i':
+	     case 'u':
+	     case 'o':
+	     case 'x':
+	     case 'X':
+	     case 'f':
+	     case 'e':
+	     case 'E':
+	     case 's':
+	     case 'p':
+	       *fmt_type=&fmt[i];
+	       break;
+	       //do not return yet. may encounter another '%'
+	     case '.':
+	       if(++dot==2)
+		 return -2;//generate error
+	       break;
+	     default:
+	       return -1;
+	     }
+
+    }
+
+  return 1;
+
+}
+
+/*****************************************************************************
+ *Function name
+ *    append_ll               : appends "ll" to the format string
+ *Input params
+ *    fmt                     : format string specified in the xml file
+ *    fmt_type                : address of the format char inside fmt
+ *
+ *  inserts "ll" just before the fmt_type
+ *
+ ****************************************************************************/
+
+void append_ll (char **fmt, char **fmt_type){
+  char *new_fmt;
+  int i;
+  int num;
+
+
+  //new_fmt = malloc(strlen(*fmt)*sizeof(char)+2);//the +2 corresponds the the "ll";
+  new_fmt = g_new(gchar, strlen(*fmt)+2);
+
+ num = *fmt_type - *fmt;
+
+ for(i=0; i<num;i++)
+   new_fmt[i] =(*fmt)[i];
+
+  new_fmt[i++] = 'l';
+  new_fmt[i++] = 'l';
+
+
+  while(i-2<strlen(*fmt))
+    {
+      new_fmt[i]=(*fmt)[i-2];
+      i++;
+    }
+
+  *fmt = new_fmt;
+  (*fmt)[i] = '\0';
+
+}
+
+
 /*****************************************************************************
  *Function name
  *    ltt_facility_open       : open facilities
@@ -299,6 +494,7 @@ void construct_fields(LttFacility *fac,
                       LttField *field,
                       field_t *fld)
 {
+  char *fmt_type;//gaby
   guint len;
   type_descriptor_t *td;
   LttType *type;
@@ -501,9 +697,187 @@ void construct_fields(LttFacility *fac,
     type->fmt = g_new(gchar, len+1);
     strcpy(type->fmt, td->fmt);
   }
-}
+    //here I should verify syntax based on type.
+    //if type is array or sequence or enum, parse_fmt.
+    //if type is basic, verify syntax, and allow or not the type format.
+    //the code can be integrated in the above code (after testing)
+
+
+
+   switch (type->type_class){
+    case LTT_ARRAY:
+    case LTT_UNION:
+    case LTT_STRUCT:
+    case LTT_SEQUENCE:
+      if(type->fmt==NULL)
+	{
+	  //Assign a default format for these complex types
+	  //type->fmt = malloc(11*sizeof(char));
+	  type->fmt = g_new(gchar, 11);
+	  type->fmt = g_strdup("{ %S, %S }");//set a default value for fmt. can directly set header and footer, but kept this way on purpose.
+	}
+      //Parse the fmt string in order to extract header, separator and footer
+      parse_fmt(type->fmt,&(type->header),&(type->separator),&(type->footer));
+
+      break;
+    case LTT_SHORT:
+    case LTT_INT:
+    case LTT_LONG:
+    case LTT_SSIZE_T:
+    case LTT_INT_FIXED:
+
+     if(type->fmt == NULL)
+	{
+	  //Assign a default format string
+	  //type->fmt = malloc(5*sizeof(char));
+	  type->fmt = g_new(gchar, 5);
+	  type->fmt=g_strdup("%lld");
+	  break;
+	}
+      else
+	if(verify_fmt_syntax((type->fmt),&fmt_type)>0)
+	  switch(fmt_type[0]){
+	  case 'd':
+	  case 'i':
+	  case 'x':
+	  case 'X':
+	    append_ll(&(type->fmt),&fmt_type);//append 'll' to fmt
+	    break;
+	  default:
+	    g_error("Format type '%c' not supported\n",fmt_type[0]);
+	    break;
+	  }
+     break;
+
+    case LTT_USHORT:
+    case LTT_UINT:
+    case LTT_ULONG:
+    case LTT_SIZE_T:
+    case LTT_OFF_T:
+    case LTT_UINT_FIXED:
+      if(type->fmt == NULL)
+	{
+	  //Assign a default format string
+	  //type->fmt= malloc(5*sizeof(char));
+	  type->fmt = g_new(gchar, 5);
+	  type->fmt=g_strdup("%lld");
+	  break;
+	}
+      else
+	if(verify_fmt_syntax((type->fmt),&fmt_type)>0)
+	  switch(fmt_type[0]){
+	  case 'd':
+	  case 'u':
+	  case 'o':
+	  case 'x':
+	  case 'X':
+	    append_ll(&(type->fmt),&fmt_type);
+	    break;
+	  default:
+	    g_error("Format type '%c' not supported\n",fmt_type[0]);
+	  }
+      break;
+
+    case LTT_CHAR:
+    case LTT_UCHAR:
+      if(type->fmt == NULL)
+	{
+	  //Assign a default format string
+	  //type->fmt = malloc(3*sizeof(char));
+	  type->fmt = g_new(gchar, 3);
+	  type->fmt = g_strdup("%c");
+	  break;
+	}
+      else
+	if(verify_fmt_syntax((type->fmt),&fmt_type)>1)
+	  switch(fmt_type[0]){
+	  case 'c':
+	  case 'd':
+	  case 'u':
+	  case 'x':
+	  case 'X':
+	  case 'o':
+	    break;
+	  default:
+	    g_error("Format type '%c' not supported\n",fmt_type[0]);
+	  }
+      break;
+
+    case LTT_FLOAT:
+      if(type->fmt == NULL)
+	{
+	  //Assign a default format string
+	  //type->fmt = malloc(3*sizeof(char));
+	  type->fmt = g_new(gchar, 3);
+	  type->fmt = g_strdup("%g");
+	  break;
+	}
+      else
+	if(verify_fmt_syntax((type->fmt),&fmt_type)>0)
+	  switch(fmt_type[0]){
+	  case 'f':
+	  case 'g':
+	  case 'G':
+	  case 'e':
+	  case 'E':
+	    break;
+	  default:
+	    g_error("Format type '%c' not supported\n",fmt_type[0]);
+	  }
+      break;
+
+    case LTT_POINTER:
+      if(type->fmt == NULL)
+	{
+	  //Assign a default format string
+	  //type->fmt = malloc(7*sizeof(char));
+	  type->fmt = g_new(gchar, 7);
+	  type->fmt = g_strdup("0x%llx");
+	  break;
+	}
+      else
+	if(verify_fmt_syntax((type->fmt),&fmt_type)>0)
+	  switch(fmt_type[0]){
+	  case 'p':
+	    //type->fmt = malloc(7*sizeof(char));
+	    type->fmt = g_new(gchar, 7);
+	    type->fmt = g_strdup("0x%llx");
+	    break;
+	  case 'x':
+	  case 'X':
+	  case 'd':
+	    append_ll(&(type->fmt),&fmt_type);
+	    break;
+	  default:
+	    g_error("Format type '%c' not supported\n",fmt_type[0]);
+	  }
+      break;
+
+    case LTT_STRING:
+      if(type->fmt == NULL)
+	{
+	  //type->fmt = malloc(7*sizeof(char));
+	  type->fmt = g_new(gchar, 5);
+	  type->fmt = g_strdup("\"%s\"");//default value for fmt.
+	  break;
+	}
+      else
+	if(verify_fmt_syntax((type->fmt),&fmt_type)>0)
+	  switch(fmt_type[0]){
+	  case 's':
+	    break;
+	  default:
+	    g_error("Format type '%c' not supported\n", fmt_type[0]);
+	  }
+      break;
+
+   default:
+     //missing enum
+     break;
+   }
 
 
+}
 
 #if 0
 void construct_types_and_fields(LttFacility * fac, type_descriptor_t * td, 
@@ -794,6 +1168,13 @@ void freeLttType(LttType * type)
   }
   if(type->fields_by_name)
     g_datalist_clear(&type->fields_by_name);
+
+  if(type->header)
+    g_free(type->header);//no need for condition? if(type->header)
+  if(type->separator)
+    g_free(type->separator);
+  if(type->footer)
+    g_free(type->footer);
 }
 
 void freeLttNamedType(LttType * type)
diff --git a/ltt/branches/poly/ltt/ltt-private.h b/ltt/branches/poly/ltt/ltt-private.h
index 2777d8bd..c565aa22 100644
--- a/ltt/branches/poly/ltt/ltt-private.h
+++ b/ltt/branches/poly/ltt/ltt-private.h
@@ -256,6 +256,10 @@ struct _LttType{
   GArray *fields;     // Array of LttFields, for array, sequence, union, struct.
   GData *fields_by_name;
   guint network;  // Is the type in network byte order ?
+  //part added by gaby for fmt:
+  char *header;
+  char *separator;
+  char *footer;
 };
 
 struct _LttEventType{
diff --git a/ltt/branches/poly/ltt/parser.c b/ltt/branches/poly/ltt/parser.c
index 0b725ecc..7ac11e7e 100644
--- a/ltt/branches/poly/ltt/parser.c
+++ b/ltt/branches/poly/ltt/parser.c
@@ -1,3 +1,6 @@
+
+
+
 /*
 
 parser.c: Generate helper declarations and functions to trace events
@@ -215,6 +218,7 @@ void getTypeAttributes(parse_file_t *in, type_descriptor_t *t,
     if(!strcmp("format",token)) {
       getEqual(in);
       t->fmt = allocAndCopy(getQuotedString(in));
+      // printf("%s - ",t->fmt);
     //} else if(!strcmp("name",token)) {
      // getEqual(in);
      // car = seekNextChar(in);
@@ -1474,7 +1478,9 @@ unsigned long getTypeChecksum(unsigned long aCrc, type_descriptor_t * type)
   crc = partial_crc32(str,crc);
   if(flag) free(str);
 
-  if(type->fmt) crc = partial_crc32(type->fmt,crc);
+  //the format string is not included in the crc calculation
+
+  //if(type->fmt) crc = partial_crc32(type->fmt,crc);
 
   if(type->type == ARRAY){
     crc = getTypeChecksum(crc,((field_t*)type->fields.array[0])->type);
diff --git a/ltt/branches/poly/ltt/type.c b/ltt/branches/poly/ltt/type.c
index d396d21a..086060a2 100644
--- a/ltt/branches/poly/ltt/type.c
+++ b/ltt/branches/poly/ltt/type.c
@@ -133,6 +133,9 @@ GQuark ltt_field_name(LttField *f)
 {
   return f->name;
 }
+
+
+
 /*****************************************************************************
  *Function name
  *    ltt_type_class : get the type class of the type
diff --git a/ltt/branches/poly/lttv/lttv/print.c b/ltt/branches/poly/lttv/lttv/print.c
index ef11b21c..d5b52086 100644
--- a/ltt/branches/poly/lttv/lttv/print.c
+++ b/ltt/branches/poly/lttv/lttv/print.c
@@ -1,4 +1,5 @@
 
+
 /* This file is part of the Linux Trace Toolkit viewer
  * Copyright (C) 2003-2004 Michel Dagenais
  *               2005 Mathieu Desnoyers
@@ -43,6 +44,9 @@
 #include <ltt/facility.h>
 #include <stdio.h>
 #include <ctype.h>
+#include<ltt/ltt-private.h>
+#include <string.h>
+
 
 void lttv_print_field(LttEvent *e, LttField *f, GString *s,
                       gboolean field_names, guint element_index) {
@@ -54,21 +58,22 @@ void lttv_print_field(LttEvent *e, LttField *f, GString *s,
   int nb, i;
 
   type = ltt_field_type(f);
+
   switch(ltt_type_class(type)) {
     case LTT_SHORT:
     case LTT_INT:
     case LTT_LONG:
     case LTT_SSIZE_T:
     case LTT_INT_FIXED:
-      if(element_index > 0)
-        g_string_append_printf(s, ", ");
       if(field_names) {
         name = ltt_field_name(f);
         if(name)
           g_string_append_printf(s, "%s = ", g_quark_to_string(name));
       }
-      g_string_append_printf(s, "%lld", ltt_event_get_long_int(e,f));
-      break;
+
+      //g_string_append_printf(s, "%lld", ltt_event_get_long_int(e,f));
+       g_string_append_printf(s, type->fmt, ltt_event_get_long_int(e,f));
+       break;
 
     case LTT_USHORT:
     case LTT_UINT:
@@ -76,14 +81,13 @@ void lttv_print_field(LttEvent *e, LttField *f, GString *s,
     case LTT_SIZE_T:
     case LTT_OFF_T:
     case LTT_UINT_FIXED:
-      if(element_index > 0)
-        g_string_append_printf(s, ", ");
       if(field_names) {
         name = ltt_field_name(f);
         if(name)
           g_string_append_printf(s, "%s = ", g_quark_to_string(name));
       }
-      g_string_append_printf(s, "%llu", ltt_event_get_long_unsigned(e,f));
+      // g_string_append_printf(s, "%llu", ltt_event_get_long_unsigned(e,f));
+      g_string_append_printf(s, type->fmt, ltt_event_get_long_unsigned(e,f));
       break;
     
     case LTT_CHAR:
@@ -101,37 +105,34 @@ void lttv_print_field(LttEvent *e, LttField *f, GString *s,
             if(name)
               g_string_append_printf(s, "%s = ", g_quark_to_string(name));
           }
-          g_string_append_printf(s, "%c", car);
+          //g_string_append_printf(s, "%c", car);
+	  g_string_append_printf(s, type->fmt, car);
         } else {
           g_string_append_printf(s, "\\%x", car);
         }
       }
       break;
     case LTT_FLOAT:
-      if(element_index > 0)
-        g_string_append_printf(s, ", ");
-      if(field_names) {
+     if(field_names) {
         name = ltt_field_name(f);
         if(name)
           g_string_append_printf(s, "%s = ", g_quark_to_string(name));
       }
-      g_string_append_printf(s, "%g", ltt_event_get_double(e,f));
+     //g_string_append_printf(s, "%g", ltt_event_get_double(e,f));
+      g_string_append_printf(s, type->fmt, ltt_event_get_double(e,f));
       break;
 
     case LTT_POINTER:
-      if(element_index > 0)
-        g_string_append_printf(s, ", ");
       if(field_names) {
         name = ltt_field_name(f);
         if(name)
           g_string_append_printf(s, "%s = ", g_quark_to_string(name));
       }
-      g_string_append_printf(s, "0x%llx", ltt_event_get_long_unsigned(e,f));
+      // g_string_append_printf(s, "0x%llx", ltt_event_get_long_unsigned(e,f));
+      g_string_append_printf(s, type->fmt, ltt_event_get_long_unsigned(e,f));
       break;
 
     case LTT_STRING:
-      if(element_index > 0)
-        g_string_append_printf(s, ", ");
       if(field_names) {
         name = ltt_field_name(f);
         if(name)
@@ -143,9 +144,6 @@ void lttv_print_field(LttEvent *e, LttField *f, GString *s,
     case LTT_ENUM:
       {
         GQuark value = ltt_enum_string_get(type, ltt_event_get_unsigned(e,f));
-        
-        if(element_index > 0)
-          g_string_append_printf(s, ", ");
         if(field_names) {
           name = ltt_field_name(f);
           if(name)
@@ -160,63 +158,76 @@ void lttv_print_field(LttEvent *e, LttField *f, GString *s,
 
     case LTT_ARRAY:
     case LTT_SEQUENCE:
-      if(element_index > 0)
-        g_string_append_printf(s, ", ");
       if(field_names) {
         name = ltt_field_name(f);
         if(name)
           g_string_append_printf(s, "%s = ", g_quark_to_string(name));
       }
-      g_string_append_printf(s, "{ ");
+      // g_string_append_printf(s, "{ ");
+      //Insert header
+      g_string_append_printf(s, type->header);//tested, works fine.
+
+
       nb = ltt_event_field_element_number(e,f);
       for(i = 0 ; i < nb ; i++) {
         LttField *child = ltt_event_field_element_select(e,f,i);
         lttv_print_field(e, child, s, field_names, i);
+	if(i<nb-1)
+	  g_string_append_printf(s,type->separator);
       }
-      g_string_append_printf(s, " }");
+      //g_string_append_printf(s, " }");
+      //Insert footer
+      g_string_append_printf(s, type->footer);//tested, works fine.
       break;
 
     case LTT_STRUCT:
-      if(element_index > 0)
-        g_string_append_printf(s, ", ");
       if(field_names) {
         name = ltt_field_name(f);
         if(name)
           g_string_append_printf(s, "%s = ", g_quark_to_string(name));
       }
-      g_string_append_printf(s, "{ ");
+      //      g_string_append_printf(s, "{ ");
+      //Insert header
+      g_string_append_printf(s, type->header);
+
       nb = ltt_type_member_number(type);
       for(i = 0 ; i < nb ; i++) {
         LttField *element;
         element = ltt_field_member(f,i);
         lttv_print_field(e, element, s, field_names, i);
+	if(i < nb-1)
+	  g_string_append_printf(s,type->separator);
       }
-      g_string_append_printf(s, " }");
+      //g_string_append_printf(s, " }");
+      //Insert footer
+      g_string_append_printf(s, type->footer);
       break;
 
     case LTT_UNION:
-      if(element_index > 0)
-        g_string_append_printf(s, ", ");
       if(field_names) {
         name = ltt_field_name(f);
         if(name)
           g_string_append_printf(s, "%s = ", g_quark_to_string(name));
       }
-      g_string_append_printf(s, "{ ");
+      //      g_string_append_printf(s, "{ ");
+      g_string_append_printf(s, type->header);
+
       nb = ltt_type_member_number(type);
       for(i = 0 ; i < nb ; i++) {
         LttField *element;
         element = ltt_field_member(f,i);
         lttv_print_field(e, element, s, field_names, i);
+	if(i<nb-1)
+	  g_string_append_printf(s, type->separator);
       }
-      g_string_append_printf(s, " }");
+      //      g_string_append_printf(s, " }");
+      g_string_append_printf(s, type->footer);
       break;
     case LTT_NONE:
       break;
   }
 }
 
-
 void lttv_event_to_string(LttEvent *e, GString *s,
     gboolean mandatory_fields, gboolean field_names, LttvTracefileState *tfs)
 { 
@@ -268,6 +279,9 @@ void lttv_event_to_string(LttEvent *e, GString *s,
   for(i=0; i<num_fields; i++) {
     field = ltt_eventtype_field(event_type, i);
     lttv_print_field(e, field, s, field_names, i);
+    //should add ',' here
+    if(i<num_fields-1)
+      g_string_append_printf(s,", ");//tested: works fine
   }
   g_string_append_printf(s, " }");
 } 
-- 
2.34.1