updates Various formatting fixes.

Sat Apr 22 15:08:01 2006  Soeren Sandmann  <sandmann@redhat.com>

	* TODO: updates
	* profile.c, sfile.c, sformat.c: Various formatting fixes.

ChangeLog

@@ -1,3 +1,8 @@
+Sat Apr 22 15:08:01 2006  Soeren Sandmann  <sandmann@redhat.com>
+
+	* TODO: updates
+	* profile.c, sfile.c, sformat.c: Various formatting fixes.
+
 Wed Apr  5 11:26:30 2006  Søren Sandmann  <sandmann@redhat.com>
 
 	* TODO: updates

TODO

@@ -30,8 +30,6 @@ Before 1.2:
 * commandline version should check that the output file is writable
   before starting the profiling.
 
-* screenshot window must be cleared when you press start.
-
 * Maybe report idle time?
 
 * Fix (deleted) problem. But more generally, whenever we can't display a
@@ -138,7 +136,8 @@ Before 1.2:
 			int id;			-> for begins  that are pointed to
 		}
 	  perhaps even with iterators. Should be compact and suitable for both
-	  input and output..
+	  input and output. As a first cut, perhaps just split out the 
+	  Instruction code.
 
 	- make the api saner; add format/content structs
 	
@@ -312,11 +311,11 @@ Later:
 	  Profiling itself it appeared that most of the time was spent
 	  in the GMarkup parser
 	- a newer version of sysprof with significantly more compact
-	  Instructions took about 5 seconds, but the profile looked
+	  Instructions structure took about 5 seconds, but the profile
-	  about the same.
+	  looked about the same.
   The difference between the two versions has to be in page faults/
-  memory speed, but the profiles looked identically. 
+  memory speed, but the profiles looked similar.
-      Try and reproduce this result in a more controlled experiment.
+      Try and reproduce this in a more controlled experiment.
 
 - See if it is possible to group the X server activity under the process that
   generated it.
@@ -495,6 +494,8 @@ Later:
 
 DONE:
 
+- screenshot window must be cleared when you press start.
+
 - Formats should become first-class, stand-alone objects that offers
   help with parsing and nothing else.
 

sfile.c

@@ -29,60 +29,6 @@
 #include "sfile.h"
 #include "sformat.h"
 
-static void
-set_error (GError **err, gint code, const char *format, va_list args)
-{
-    char *msg;
-    
-    if (!err)
-        return;
-    
-    msg = g_strdup_vprintf (format, args);
-    
-    if (*err == NULL)
-    {
-        *err = g_error_new_literal (G_MARKUP_ERROR, code, msg);
-    }
-    else
-    {
-        /* Warning text from GLib */
-        g_warning ("GError set over the top of a previous GError or uninitialized memory.\n" 
-                   "This indicates a bug in someone's code. You must ensure an error is NULL before it's set.\n"
-                   "The overwriting error message was: %s",
-                   msg);
-    }
-    
-    g_free (msg);
-}
-
-static void
-set_unknown_element_error (GError **err, const char *format, ...)
-{
-    va_list args;
-    va_start (args, format);
-    set_error (err, G_MARKUP_ERROR_UNKNOWN_ELEMENT, format, args);
-    va_end (args);
-}
-
-static void
-set_unknown_attribute_error (GError **err, const char *format, ...)
-{
-    va_list args;
-    va_start (args, format);
-    set_error (err, G_MARKUP_ERROR_UNKNOWN_ATTRIBUTE, format, args);
-    va_end (args);
-}
-
-static void
-set_invalid_content_error (GError **err, const char *format, ...)
-{
-    va_list args;
-    va_start (args, format);
-    set_error (err, G_MARKUP_ERROR_INVALID_CONTENT, format, args);
-    va_end (args);
-}
-
-/* reading */
 typedef struct BuildContext BuildContext;
 typedef struct Instruction Instruction;
 typedef enum
@@ -137,6 +83,63 @@ struct BuildContext
     GArray *instructions;
 };
 
+static void
+set_error (GError **err,
+	   gint code,
+	   const char *format,
+	   va_list args)
+{
+    char *msg;
+    
+    if (!err)
+        return;
+    
+    msg = g_strdup_vprintf (format, args);
+    
+    if (*err == NULL)
+    {
+        *err = g_error_new_literal (G_MARKUP_ERROR, code, msg);
+    }
+    else
+    {
+        /* Warning text from GLib */
+        g_warning ("GError set over the top of a previous GError or uninitialized memory.\n" 
+                   "This indicates a bug in someone's code. You must ensure an error is NULL before it's set.\n"
+                   "The overwriting error message was: %s",
+                   msg);
+    }
+    
+    g_free (msg);
+}
+
+static void
+set_unknown_element_error (GError **err, const char *format, ...)
+{
+    va_list args;
+    va_start (args, format);
+    set_error (err, G_MARKUP_ERROR_UNKNOWN_ELEMENT, format, args);
+    va_end (args);
+}
+
+static void
+set_unknown_attribute_error (GError **err, const char *format, ...)
+{
+    va_list args;
+    va_start (args, format);
+    set_error (err, G_MARKUP_ERROR_UNKNOWN_ATTRIBUTE, format, args);
+    va_end (args);
+}
+
+static void
+set_invalid_content_error (GError **err, const char *format, ...)
+{
+    va_list args;
+    va_start (args, format);
+    set_error (err, G_MARKUP_ERROR_INVALID_CONTENT, format, args);
+    va_end (args);
+}
+
+/* reading */
 static gboolean
 is_all_blank (const char *text)
 {
@@ -259,15 +262,11 @@ hook_up_pointers (SFileInput *file)
 {
     int i;
     
-#if 0
-    g_print ("emfle\n");
-#endif
     for (i = 0; i < file->n_instructions; ++i)
     {
         Instruction *instruction = &(file->instructions[i]);
 	
-        if (instruction->kind == VALUE &&
+        if (stype_is_pointer (instruction->type))
-            stype_is_pointer (instruction->type))
         {
             gpointer target_object;
             Instruction *target_instruction;
@@ -279,10 +278,6 @@ hook_up_pointers (SFileInput *file)
             else
                 target_object = NULL;
 	    
-#if 0
-            g_print ("target object: %p\n", target_object);
-#endif
-	    
             *(instruction->u.pointer.location) = target_object;
         }
     }
@@ -489,7 +484,7 @@ free_instructions (Instruction *instructions, int n_instructions)
     {
         Instruction *instruction = &(instructions[i]);
 	
-        if (instruction->kind == VALUE && stype_is_string (instruction->type))
+        if (stype_is_string (instruction->type))
             g_free (instruction->u.string.value);
     }
     
@@ -536,7 +531,9 @@ process_instruction_pairs (Instruction *first)
 }
 
 static gboolean
-post_process_read_instructions (Instruction *instructions, int n_instructions, GError **err)
+post_process_read_instructions (Instruction  *instructions,
+				int           n_instructions,
+				GError      **err)
 {
     gboolean retval = TRUE;
     GHashTable *instructions_by_id;
@@ -565,8 +562,7 @@ post_process_read_instructions (Instruction *instructions, int n_instructions, G
     {
         Instruction *instruction = &(instructions[i]);
 	
-        if (instruction->kind == VALUE &&
+        if (stype_is_pointer (instruction->type))
-            stype_is_pointer (instruction->type))
         {
             int target_id = instruction->u.pointer.target_id;
 	    
@@ -610,7 +606,10 @@ post_process_read_instructions (Instruction *instructions, int n_instructions, G
 }
 
 static Instruction *
-build_instructions (const char *contents, SFormat *format, int *n_instructions, GError **err)
+build_instructions (const char *contents,
+		    SFormat    *format,
+		    int        *n_instructions,
+		    GError    **err)
 {
     BuildContext build;
     GMarkupParseContext *parse_context;
@@ -898,7 +897,7 @@ post_process_write_instructions (SFileOutput *sfile)
                 if (!target)
                 {
                     g_warning ("pointer has unknown target\n");
-                    return;
+		    goto out;
                 }
 		
                 g_assert (target->kind == END);
@@ -916,10 +915,13 @@ post_process_write_instructions (SFileOutput *sfile)
         }
     }
 
+out:
+    g_hash_table_destroy (instructions_by_object);
 }
 
 static void
-add_indent (GString *output, int indent)
+add_indent (GString *output,
+	    int      indent)
 {
     int i;
     
@@ -928,13 +930,15 @@ add_indent (GString *output, int indent)
 }
 
 static void
-add_integer (GString *output, int value)
+add_integer (GString *output,
+	     int      value)
 {
     g_string_append_printf (output, "%d", value);
 }
 
 static void
-add_string (GString *output, const char *str)
+add_string (GString    *output,
+	    const char *str)
 {
     char *escaped = g_markup_escape_text (str, -1);
     g_string_append_c (output, '\"');
@@ -944,7 +948,10 @@ add_string (GString *output, const char *str)
 }
 
 static void
-add_begin_tag (GString *output, int indent, const char *name, int id)
+add_begin_tag (GString    *output,
+	       int         indent,
+	       const char *name,
+	       int         id)
 {
     add_indent (output, indent);
     
@@ -955,7 +962,9 @@ add_begin_tag (GString *output, int indent, const char *name, int id)
 }
 
 static void
-add_end_tag (GString *output, int indent, const char *name)
+add_end_tag (GString    *output,
+	     int         indent,
+	     const char *name)
 {
     add_indent (output, indent);
     g_string_append_printf (output, "</%s>", name);
@@ -1101,7 +1110,9 @@ sfile_output_save (SFileOutput  *sfile,
 	    {
                 add_string (output, instruction->u.string.value);
             }
+
             add_end_tag (output, 0, get_name (instruction));
+
 	    add_nl (output);
             break;
         }

sformat.c

@@ -62,51 +62,6 @@ struct SType
 
 static void type_free (SType *type);
 
-
-gboolean
-stype_is_record (SType *type)
-{
-    return type->kind == TYPE_RECORD;
-}
-
-gboolean
-stype_is_list (SType *type)
-{
-    return type->kind == TYPE_LIST;
-}
-
-gboolean
-stype_is_pointer (SType *type)
-{
-    return type->kind == TYPE_POINTER;
-}
-
-gboolean
-stype_is_integer (SType *type)
-{
-    return type->kind == TYPE_INTEGER;
-}
-
-gboolean
-stype_is_string (SType *type)
-{
-    return type->kind == TYPE_STRING;
-}
-
-SType *
-stype_get_target_type (SType *type)
-{
-    g_return_val_if_fail (stype_is_pointer (type), NULL);
-    
-    return type->target;
-}
-
-const char *
-stype_get_name (SType *type)
-{
-    return type->name;
-}
-
 /*
  * Transition
  */
@@ -179,6 +134,7 @@ sformat_free (SFormat *format)
 	
 	type_free (type);
     }
+    g_queue_free (format->types);
     
     for (list = format->states->head; list; list = list->next)
     {
@@ -186,6 +142,7 @@ sformat_free (SFormat *format)
 	
 	state_free (state);
     }
+    g_queue_free (format->states);
     
     for (list = format->transitions->head; list; list = list->next)
     {
@@ -193,6 +150,7 @@ sformat_free (SFormat *format)
 	
 	transition_free (transition);
     }
+    g_queue_free (format->transitions);
     
     g_free (format);
 }
@@ -206,8 +164,6 @@ sformat_set_type (SFormat *format,
     
     state_add_transition (format->begin, type->enter);
     transition_set_to_state (type->exit, format->end);
-    
-    /* Fix up pointer types */
 }
 
 /*
@@ -231,6 +187,46 @@ type_new (SFormat *format,
     return type;
 }
 
+static SType *
+type_new_from_forward (SFormat *format,
+		       TypeKind kind,
+		       const char *name,
+		       SForward *forward)
+{
+    SType *type;
+    
+    if (forward)
+    {
+	type = (SType *)forward;
+	type->kind = kind;
+	type->name = g_strdup (name);
+    }
+    else
+    {
+	type = type_new (format, kind, name);
+    }
+    
+    return type;
+}
+
+
+static SType *
+type_new_value (SFormat *format, TypeKind kind, const char *name)
+{
+    SType *type = type_new (format, kind, name);
+    State *before, *after;
+    Transition *value;
+    
+    before = state_new (format);
+    after = state_new (format);
+    
+    type->enter = transition_new (format, BEGIN, type, NULL, before);
+    type->exit  = transition_new (format, END, type, after, NULL);
+    value = transition_new (format, VALUE, type, before, after);
+    
+    return type;
+}
+
 static void
 type_free (SType *type)
 {
@@ -238,8 +234,17 @@ type_free (SType *type)
     g_free (type);
 }
 
+SForward *
+sformat_declare_forward (SFormat *format)
+{
+    SType *type = type_new (format, TYPE_FORWARD, NULL);
+    
+    return (SForward *)type;
+}
+
+
 static GQueue *
-type_queue (SType *content1,
+expand_varargs (SType *content1,
 		va_list args)
 {
     GQueue *types = g_queue_new ();
@@ -257,15 +262,6 @@ type_queue (SType *content1,
     return types;
 }
 
-SForward *
-sformat_declare_forward (SFormat *format)
-{
-    SType *type = type_new (format, TYPE_FORWARD, NULL);
-    
-    return (SForward *)type;
-}
-
-
 SType *
 sformat_make_record (SFormat *format,
 		     const char *name,
@@ -281,7 +277,7 @@ sformat_make_record (SFormat *format,
     
     /* Build queue of child types */
     va_start (args, content);
-    types = type_queue (content, args);
+    types = expand_varargs (content, args);
     va_end (args);
     
     /* chain types together */
@@ -300,18 +296,8 @@ sformat_make_record (SFormat *format,
     
     g_queue_free (types);
     
-    /* Return resulting fragment */
+    /* create and return the new type */
-    if (forward)
+    type = type_new_from_forward (format, TYPE_RECORD, name, forward);
-    {
-	type = (SType *)forward;
-	type->kind = TYPE_RECORD;
-	type->name = g_strdup (name);
-    }
-    else
-    {
-	type = type_new (format, TYPE_RECORD, name);
-    }
-    
     type->enter = transition_new (format, BEGIN, type, NULL, begin);
     type->exit = transition_new (format, END, type, state, NULL);
     
@@ -327,16 +313,7 @@ sformat_make_list   (SFormat *format,
     SType *type;
     State *list_state;
 
-    if (forward)
+    type = type_new_from_forward (format, TYPE_LIST, name, forward);
-    {
-	type = (SType *)forward;
-	type->kind = TYPE_LIST;
-	type->name = g_strdup (name);
-    }
-    else
-    {
-	type = type_new (format, TYPE_LIST, name);
-    }
 
     list_state = state_new (format);
     
@@ -349,23 +326,6 @@ sformat_make_list   (SFormat *format,
     return type;
 }
 
-static SType *
-type_new_value (SFormat *format, TypeKind kind, const char *name)
-{
-    SType *type = type_new (format, kind, name);
-    State *before, *after;
-    Transition *value;
-    
-    before = state_new (format);
-    after = state_new (format);
-    
-    type->enter = transition_new (format, BEGIN, type, NULL, before);
-    type->exit  = transition_new (format, END, type, after, NULL);
-    value = transition_new (format, VALUE, type, before, after);
-    
-    return type;
-}
-
 SType *
 sformat_make_pointer (SFormat     *format,
 		      const char  *name,
@@ -392,6 +352,51 @@ sformat_make_string  (SFormat *format,
 }
 
 
+
+gboolean
+stype_is_record (SType *type)
+{
+    return type->kind == TYPE_RECORD;
+}
+
+gboolean
+stype_is_list (SType *type)
+{
+    return type->kind == TYPE_LIST;
+}
+
+gboolean
+stype_is_pointer (SType *type)
+{
+    return type->kind == TYPE_POINTER;
+}
+
+gboolean
+stype_is_integer (SType *type)
+{
+    return type->kind == TYPE_INTEGER;
+}
+
+gboolean
+stype_is_string (SType *type)
+{
+    return type->kind == TYPE_STRING;
+}
+
+SType *
+stype_get_target_type (SType *type)
+{
+    g_return_val_if_fail (stype_is_pointer (type), NULL);
+    
+    return type->target;
+}
+
+const char *
+stype_get_name (SType *type)
+{
+    return type->name;
+}
+
 /* Consider adding unions at some point
  *
  * To be useful they should probably be anonymous, so that
@@ -414,6 +419,8 @@ sformat_make_string  (SFormat *format,
  * elements. That will make things a lot easier, and unions are
  * still pretty useful if you put big things like lists in them.
  *
+ * Or maybe just give them a name ...
+ *
  * We may also consider adding anonymous records. These will
  * not be able to have pointers associated with them though
  * (because there wouldn't be a natural place