GNOME.org
 

Re: glib: changes to the g*_array API.

First, note that the order preservation only applies to the pointer
array.  And as to code breakage, I have a lot of code outside of the
gnome CVS repository using the interface, and I'd rather not break it.

I intentionally made the interface the way it is.  It is most efficient,
of course, to not preserve order.

I think that the option to preserve order should not be implemented as
a new argument.  Instead, I think there should be two functions so that
the meaning is clear from reading each usage and to avoid breaking existing
code.  The option can further be added to the g_array class of functions
as well, and there are currently no remove functions implemented for that
class so there will be no breakage.

Now that we've decided there should be two behaviors, and I hope we can
agree on using two functions, the decision remains as to the default.  We
can have

	g_ptr_array_remove_index_preserve
	g_ptr_array_remove_index

or

	g_ptr_array_remove_index_no_preserve
        g_ptr_array_remove_index

or

        g_ptr_array_remove_index_preserve
        g_ptr_array_remove_index_no_preserve

I'm okay with either of the first two, I don't like the verbosity of
the third, and clearly the first set prevents me from breaking my 
existing code, but am willing to compromise.  

I think that all three of the array classes should have an identical
interface.  It is currently not this way because of the particular
order in which code of mine got merged into the main glib line and
in which people started using the interfaces (thereby preventing
me from cleaning up older interfaces).  While we're at it, some time
ago I changed all of the g_*array_truncate function to be named
g_*array_set_size, and I'd like to change the name of g_string_truncate
to be in line with the others.  Thoughts?  There are a few other nit-picks
I have with the glib interface which are going to be difficult to change
at this point, but still may be nice: the inconsistent usage of _free and 
_destroy is one of them, and I can't remember the others.

-josh


Quoting Sebastian Wilhelmi (wilhelmi@ira.uka.de):
> Hi, all
> 
> I hereby propose to change the g*_array API:
> 
> The current g_ptr_array_remove[_index] functions work a little
> suprisingly, as they do not preserve order. So I added the argument
> gboolean preserve_order.
> 
> This will of course break tons of code: namely, lets see,
> cvs.gnome.org/lxr -> search for g_ptr_array_remove -> uh,oh only one
> program, /ORBit/src/services/name/orbit-name-server.c, written by me (what
> a coincidence ;-), which I'll change then. (actually, as you all guess, I
> might need it there).
> 
> Additionally I added the functions g_array_remove_index and
> g_byte_array_remove_index with a similar prototype to make it consistent
> and to prevent the people using arrays form doing this on their own. To
> cite somebody, whos name I forgot: 'Off by one' is the most common
> mistake, and doing the memmove stuff on ones own all over the place is
> asking for trouble. 
> 
> Anyway, here comes the patch: If approved, I'll apply this (i.e. I do have
> cvs write acces), but of course I'm asking first
> 
> Bye, Sebastian
> -- 
> +--------------============####################============--------------+
> | Sebastian Wilhelmi, Institute for Computer Design and Fault Tolerance, |
> | University of Karlsruhe;  Building 20.20, Room 263,  D-76128 Karlsruhe |
> | mail: wilhelmi@ira.uka.de;  fax: +49 721 370455;  fon: +49 721 6084353 |
> +-----------------> http://goethe.ira.uka.de/~wilhelmi <-----------------+

> Index: ChangeLog
> ===================================================================
> RCS file: /cvs/gnome/glib/ChangeLog,v
> retrieving revision 1.88
> diff -u -r1.88 ChangeLog
> --- ChangeLog	1998/10/28 01:32:50	1.88
> +++ ChangeLog	1998/10/28 10:20:08
> @@ -1,3 +1,16 @@
> +1998-10-28  Sebastian Wilhelmi  <wilhelmi@ira.uka.de>
> +
> +	* glib.h: 
> +	* garray.h:
> +	(g_array_remove_index): new function for removing an entry from an
> +	array. preserver_order determines, if the last element is moved to
> +	the position of the removed one (=FALSE), or if all the remaining
> +	elements are moved one position down (=TRUE).
> +	(g_ptr_array_remove_index, g_ptr_array_remove): added
> +	preserver_order argument working like explained above
> +	(g_byte_array_remove_index): new function; byte_array wrapper for
> +	g_array_remove_index
> +
>  Tue Oct 27 07:25:53 1998  Tim Janik  <timj@gtk.org>
>  
>  	* glib.h:
> Index: garray.c
> ===================================================================
> RCS file: /cvs/gnome/glib/garray.c,v
> retrieving revision 1.7
> diff -u -r1.7 garray.c
> --- garray.c	1998/09/07 09:43:46	1.7
> +++ garray.c	1998/10/28 10:20:09
> @@ -126,6 +126,38 @@
>    return farray;
>  }
>  
> +GArray*
> +g_array_remove_index (GArray* farray,
> +		      guint index,
> +		      gboolean preserve_order)
> +{
> +  GRealArray* array = (GRealArray*) farray;
> +
> +  g_return_val_if_fail (array, NULL);
> +
> +  g_return_val_if_fail (index >= 0 && index < array->len, NULL);
> +
> +  if (index != array->len - 1)
> +    {
> +      if (preserve_order)
> +	g_memmove (array->data + array->elt_size * index, 
> +		   array->data + array->elt_size * (index + 1), 
> +		   array->elt_size * (array->len - index - 1));
> +      else
> +	g_memmove (array->data + array->elt_size * index, 
> +		   array->data + array->elt_size * (array->len - 1), 
> +		   array->elt_size);
> +    }
> +  
> +  if (array->zero_terminated)
> +    memset (array->data + array->elt_size * (array->len - 1), 0, 
> +	    array->elt_size);
> +
> +  array->len -= 1;
> +
> +  return farray;
> +}
> +
>  static gint
>  g_nearest_pow (gint num)
>  {
> @@ -245,7 +277,8 @@
>  
>  gpointer
>  g_ptr_array_remove_index (GPtrArray* farray,
> -			  gint index)
> +			  guint index,
> +			  gboolean preserve_order)
>  {
>    GRealPtrArray* array = (GRealPtrArray*) farray;
>    gpointer result;
> @@ -255,8 +288,15 @@
>    g_return_val_if_fail (index >= 0 && index < array->len, NULL);
>  
>    result = array->pdata[index];
> -
> -  array->pdata[index] = array->pdata[array->len - 1];
> +  
> +  if (index != array->len - 1)
> +    {
> +      if (preserve_order)
> +	g_memmove (array->pdata + index, array->pdata + index + 1, 
> +		   array->len - index - 1);
> +      else
> +	array->pdata[index] = array->pdata[array->len - 1];
> +    }
>  
>    array->pdata[array->len - 1] = NULL;
>  
> @@ -267,7 +307,8 @@
>  
>  gboolean
>  g_ptr_array_remove (GPtrArray* farray,
> -		    gpointer data)
> +		    gpointer data,
> +		    gboolean preserve_order)
>  {
>    GRealPtrArray* array = (GRealPtrArray*) farray;
>    int i;
> @@ -278,7 +319,7 @@
>      {
>        if (array->pdata[i] == data)
>  	{
> -	  g_ptr_array_remove_index (farray, i);
> +	  g_ptr_array_remove_index (farray, i, preserve_order);
>  	  return TRUE;
>  	}
>      }
> @@ -335,6 +376,15 @@
>  				   guint       length)
>  {
>    g_array_set_size ((GArray*) array, length);
> +
> +  return array;
> +}
> +
> +GByteArray* g_byte_array_remove_index (GByteArray *array,
> +				       guint index,
> +				       gboolean preserve_order)
> +{
> +  g_array_remove_index((GArray*) array, index, preserve_order);
>  
>    return array;
>  }
> Index: glib.h
> ===================================================================
> RCS file: /cvs/gnome/glib/glib.h,v
> retrieving revision 1.63
> diff -u -r1.63 glib.h
> --- glib.h	1998/10/28 01:32:53	1.63
> +++ glib.h	1998/10/28 10:20:14
> @@ -1800,6 +1800,9 @@
>  			      guint	    len);
>  GArray* g_array_set_size     (GArray	   *array,
>  			      guint	    length);
> +GArray* g_array_remove_index (GArray	   *array,
> +			      guint	    index,
> +			      gboolean	    preserve_order);
>  
>  /* Resizable pointer array.  This interface is much less complicated
>   * than the above.  Add appends appends a pointer.  Remove fills any
> @@ -1812,9 +1815,11 @@
>  void	    g_ptr_array_set_size	   (GPtrArray	*array,
>  					    gint	 length);
>  gpointer    g_ptr_array_remove_index	   (GPtrArray	*array,
> -					    gint	 index);
> +					    guint	 index,
> +					    gboolean 	 preserve_order);
>  gboolean    g_ptr_array_remove		   (GPtrArray	*array,
> -					    gpointer	 data);
> +					    gpointer	 data,
> +					    gboolean 	 preserve_order);
>  void	    g_ptr_array_add		   (GPtrArray	*array,
>  					    gpointer	 data);
>  
> @@ -1822,17 +1827,20 @@
>   * but type-safe.
>   */
>  
> -GByteArray* g_byte_array_new	  (void);
> -void	    g_byte_array_free	  (GByteArray	*array,
> -				   gboolean	 free_segment);
> -GByteArray* g_byte_array_append	  (GByteArray	*array,
> -				   const guint8 *data,
> -				   guint	 len);
> -GByteArray* g_byte_array_prepend  (GByteArray	*array,
> -				   const guint8 *data,
> -				   guint	 len);
> -GByteArray* g_byte_array_set_size (GByteArray	*array,
> -				   guint	 length);
> +GByteArray* g_byte_array_new	      (void);
> +void	    g_byte_array_free	      (GByteArray	*array,
> +				       gboolean 	 free_segment);
> +GByteArray* g_byte_array_append	      (GByteArray	*array,
> +				       const guint8 	*data,
> +				       guint	 	 len);
> +GByteArray* g_byte_array_prepend      (GByteArray	*array,
> +				       const guint8 	*data,
> +				       guint	 	 len);
> +GByteArray* g_byte_array_set_size     (GByteArray	*array,
> +				       guint	 	 length);
> +GByteArray* g_byte_array_remove_index (GByteArray 	*array,
> +				       guint 		 index,
> +				       gboolean 	 preserve_order);
>  
>  
>  /* Hash Functions


-- 
-josh
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]