[libcroco-list] minor: cr_declaration_destroy cleanup

[Peter Moulder <Peter Moulder infotech monash edu au>]

The existing cr_declaration_destroy implementation makes it a bit hard
to see that the right things are getting freed.  The version in the
patch below makes it clearer.  The patch makes no functional change
(assuming order of freeing makes no difference) so long as the linked
list prev/next fields are filled in correctly.  (If the prev/next fields
aren't consistent, then there's a functional change of throwing an
assertion failure rather than trusting the fields as set.)

I usually use "unified" diff format (diff -u/-Unn), but this patch is
easier to read in diff -C8 format: look at the two versions of
cr_declaration_destroy as a whole rather than looking at individual
additions/deletions/changes.

I can't remember whether g_free(NULL) has always been fine, but note
that libcroco's configure.in checks for glib >= v2, and I think
g_free(NULL) has been fine for at least a very long time.
glib documentation of g_free doesn't mention any version compatibility
issues.


As an independent change, the attached patch also adds to the
documentation of cr_declaration_new.


I'll commit in a few days.  It's of course low priority, I can delay
committing if anyone wants.

pjrm.


Index: cr-declaration.c
===================================================================
*** cr-declaration.c	(révision 314)
--- cr-declaration.c	(copie de travail)
*************** dump (CRDeclaration * a_this, FILE * a_f
*** 60,75 ****
--- 60,79 ----
   * cr_declaration_new:
   * @a_statement: the statement this declaration belongs to. can be NULL.
   * a_property: the property string of the declaration
   * a_value: the value expression of the declaration.
   *Constructor of #CRDeclaration.
   *
   *Returns the newly built instance of #CRDeclaration, or NULL in
   *case of error.
+  *
+  *The returned CRDeclaration takes ownership of @a_property and @a_value.
+  *(E.g. cr_declaration_destroy on this CRDeclaration will also free
+  * a_property and @a_value.)
   */
  CRDeclaration *
  cr_declaration_new (CRStatement * a_statement,
                      CRString * a_property, CRTerm * a_value)
  {
          CRDeclaration *result = NULL;
  
          g_return_val_if_fail (a_property, NULL);
*************** cr_declaration_unref (CRDeclaration * a_
*** 764,822 ****
  void
  cr_declaration_destroy (CRDeclaration * a_this)
  {
          CRDeclaration *cur = NULL;
  
          g_return_if_fail (a_this);
  
          /*
!          *Go get the tail of the list.
!          *Meanwhile, free each property/value pair contained in the list.
           */
!         for (cur = a_this; cur && cur->next; cur = cur->next) {
!                 if (cur->property) {
!                         cr_string_destroy (cur->property);
!                         cur->property = NULL;
!                 }
  
!                 if (cur->value) {
!                         cr_term_destroy (cur->value);
!                         cur->value = NULL;
!                 }
!         }
  
-         if (cur) {
                  if (cur->property) {
                          cr_string_destroy (cur->property);
                          cur->property = NULL;
                  }
  
                  if (cur->value) {
                          cr_term_destroy (cur->value);
                          cur->value = NULL;
                  }
          }
  
!         /*in case the list contains only one element */
!         if (cur && !cur->prev) {
!                 g_free (cur);
!                 return;
!         }
! 
!         /*walk backward the list and free each "next" element */
!         for (cur = cur->prev; cur && cur->prev; cur = cur->prev) {
!                 if (cur->next) {
!                         g_free (cur->next);
!                         cur->next = NULL;
!                 }
!         }
! 
!         if (!cur)
!                 return;
! 
!         if (cur->next) {
!                 g_free (cur->next);
!                 cur->next = NULL;
!         }
! 
!         g_free (cur);
  }
--- 768,801 ----
  void
  cr_declaration_destroy (CRDeclaration * a_this)
  {
          CRDeclaration *cur = NULL;
  
          g_return_if_fail (a_this);
  
          /*
!          * Go to the last element of the list.
           */
!         for (cur = a_this; cur->next; cur = cur->next)
!                 g_assert (cur->next->prev == cur);
  
!         /*
!          * Walk backward the list and free each "next" element.
!          * Meanwhile, free each property/value pair contained in the list.
!          */
!         for (; cur; cur = cur->prev) {
!                 g_free (cur->next);
!                 cur->next = NULL;
  
                  if (cur->property) {
                          cr_string_destroy (cur->property);
                          cur->property = NULL;
                  }
  
                  if (cur->value) {
                          cr_term_destroy (cur->value);
                          cur->value = NULL;
                  }
          }
  
!         g_free (a_this);
  }