Re: TODO for GLib 1.4

[Sebastian Wilhelmi <wilhelmi ira uka de>]

Hi Owen,

> > > I would disagree with this. Basically, it makes things more
> > > complicated to get marginal gains in a few cases. I think we should
> > > make things simpler for our users by paying attention to worst-case
> > > performance. A frozen hash table has HORRIBLE worst-case performance.
> > >
> > > If it made things 10 times faster, then, yes, I would say that it
> > > would be worth it. But the speedups you have here seem to be order
> > > 20%.
> >
> > So you propose to remove the _free and _thaw functions alltogether. Thats
> > fine with me. Should there still be the functions for compatibility or
> > should I just remove them?
> 
> Hmmm, I think for GLib 1.4 we should leave the functions there,
> but make them do nothing, and mark them in the source and documentation
> as deprecated.

Ok, appended is a patch to do that. If --enable-debug is given to configure,
this even spits out a warning (at mot one), if the functions are used. I also
marked the g_scanner_(freeze|thaw)_symbol_table functions as deprecated, as
all they do it call g_hash_table_(freeze|thaw). 

> We might want to create a glibcompat.h (like gtkcompat.h)
> with stuff that can be disabled with -DG_DISABLE_COMPAT_H

I don't think, that buys us much.
 
> > > > * Regarding Bug #5097. I would just change g_basename to return a
> > > > string, that must be freed. This of course could open memory
> > > > holes. So an alternative might be to rename the function to
> > > > g_basename2() with the new semantics, remove the old one and rename
> > > > the function back to g_basename in GLib 1.6, such that everyone
> > > > using the function is aware of the change.  --> nothing.  *
> > >
> > > Hmmmm, that would still break everybody's code in the 1.4 => 1.6
> > > transition.
> >
> > Do you mean the 1.2=>1.6 transition here? Will such a transition happen?
> > Based on current experience 1.6 will be out in 2 years. Who will change
> > versions that late?
> 
> No, I mean, the 1.4 => 1.6 transition.
> 
> At that point, those people using g_basename2() will have to
> convert their code to use g_basename().

Ok, that is not very nice, I agree.

> > > I would say that the right way to fix this would be
> > > to introduce, say:
> > >
> > >  g_path_get_basename()
> > >
> > > that does the right thing, and deprecate g_basename. I'm just not sure
> > > that this is enough of a problem to be worth trying to fix.
> >
> > My proposal actually would be to break the semantics of the function and
> > make a big warning into the transition file and the documentation and fix
> > all occurences in cvs.gnome.org
> 
> I really don't like breaking the semantics of a function unless the
> compiler catches it. I guess you mean to do this by removing
> g_basename(), but if we do this, I'd rather use a pretty name that
> we can leave in place, instead of something like g_basename2() that
> we plan to change later.

Ok, I added the functions g_path_get_basename/g_path_get_dirname. Also I
deprecated g_basename and g_dirname (like above), but both continue to do the
things, they did so far. g_path_get_dirname does the same as g_dirname, the
renaming is only for consistency. The patch is attached.

Bye,
Sebastian
-- 
Sebastian Wilhelmi                   |            här ovanför alla molnen
mailto:wilhelmi@ira.uka.de           |     är himmlen så förunderligt blå
http://goethe.ira.uka.de/~wilhelmi   |
Index: ghash.c
===================================================================
RCS file: /cvs/gnome/glib/ghash.c,v
retrieving revision 1.18
diff -u -b -B -r1.18 ghash.c
--- ghash.c	2000/04/17 13:23:27	1.18
+++ ghash.c	2000/04/27 08:42:16
@@ -48,7 +48,6 @@
 {
   gint size;
   gint nnodes;
-  guint frozen;
   GHashNode **nodes;
   GHashFunc hash_func;
   GCompareFunc key_compare_func;
@@ -80,7 +79,6 @@
   hash_table = g_new (GHashTable, 1);
   hash_table->size = HASH_TABLE_MIN_SIZE;
   hash_table->nnodes = 0;
-  hash_table->frozen = FALSE;
   hash_table->hash_func = hash_func ? hash_func : g_direct_hash;
   hash_table->key_compare_func = key_compare_func;
   hash_table->nodes = g_new (GHashNode*, hash_table->size);
@@ -167,7 +165,6 @@
     {
       *node = g_hash_node_new (key, value);
       hash_table->nnodes++;
-      if (!hash_table->frozen)
 	g_hash_table_resize (hash_table);
     }
 }
@@ -189,7 +186,6 @@
       g_hash_node_destroy (dest);
       hash_table->nnodes--;
   
-      if (!hash_table->frozen)
         g_hash_table_resize (hash_table);
     }
 }
@@ -221,19 +217,20 @@
 void
 g_hash_table_freeze (GHashTable *hash_table)
 {
-  g_return_if_fail (hash_table != NULL);
+#ifdef G_ENABLE_DEBUG
+  static gboolean first_call = TRUE;
   
-  hash_table->frozen++;
+  if (first_call)
+    {
+      g_warning("g_hash_table_freeze and g_hash_table_thaw are deprecated.");
+      first_call = FALSE;
+    }
+#endif /* G_ENABLE_DEBUG */
 }
 
 void
 g_hash_table_thaw (GHashTable *hash_table)
 {
-  g_return_if_fail (hash_table != NULL);
-  
-  if (hash_table->frozen)
-    if (!(--hash_table->frozen))
-      g_hash_table_resize (hash_table);
 }
 
 guint
@@ -278,7 +275,6 @@
 	}
     }
   
-  if (!hash_table->frozen)
     g_hash_table_resize (hash_table);
   
   return deleted;
Index: glib.h
===================================================================
RCS file: /cvs/gnome/glib/glib.h,v
retrieving revision 1.165
diff -u -b -B -r1.165 glib.h
--- glib.h	2000/04/26 08:42:19	1.165
+++ glib.h	2000/04/27 08:42:17
@@ -1092,8 +1092,6 @@
 					 gconstpointer	 lookup_key,
 					 gpointer	*orig_key,
 					 gpointer	*value);
-void	    g_hash_table_freeze		(GHashTable	*hash_table);
-void	    g_hash_table_thaw		(GHashTable	*hash_table);
 void	    g_hash_table_foreach	(GHashTable	*hash_table,
 					 GHFunc		 func,
 					 gpointer	 user_data);
@@ -1102,6 +1100,10 @@
 					 gpointer	 user_data);
 guint	    g_hash_table_size		(GHashTable	*hash_table);
 
+/* The following two functions are deprecated and will be removed in
+ * the next major release. They do no good. */
+void	    g_hash_table_freeze		(GHashTable	*hash_table);
+void	    g_hash_table_thaw		(GHashTable	*hash_table);
 
 /* Caches
  */
@@ -2240,8 +2242,6 @@
 						 gpointer	 user_data);
 gpointer	g_scanner_lookup_symbol		(GScanner	*scanner,
 						 const gchar	*symbol);
-void		g_scanner_freeze_symbol_table	(GScanner	*scanner);
-void		g_scanner_thaw_symbol_table	(GScanner	*scanner);
 void		g_scanner_unexp_token		(GScanner	*scanner,
 						 GTokenType	expected_token,
 						 const gchar	*identifier_spec,
@@ -2267,6 +2267,10 @@
   g_scanner_scope_foreach_symbol ((scanner), 0, (func), (data)); \
 } G_STMT_END
 
+/* The following two functions are deprecated and will be removed in
+ * the next major release. They do no good. */
+void		g_scanner_freeze_symbol_table	(GScanner	*scanner);
+void		g_scanner_thaw_symbol_table	(GScanner	*scanner);
 
 /* GCompletion
  */
Index: gscanner.c
===================================================================
RCS file: /cvs/gnome/glib/gscanner.c,v
retrieving revision 1.23
diff -u -b -B -r1.23 gscanner.c
--- gscanner.c	2000/04/19 06:49:39	1.23
+++ gscanner.c	2000/04/27 08:42:17
@@ -564,17 +564,21 @@
 void
 g_scanner_freeze_symbol_table (GScanner *scanner)
 {
-  g_return_if_fail (scanner != NULL);
+#ifdef G_ENABLE_DEBUG
+  static gboolean first_call = TRUE;
   
-  g_hash_table_freeze (scanner->symbol_table);
+  if (first_call)
+    {
+      g_warning("g_scanner_freeze_symbol_table and "
+		"g_scanner_thaw_symbol_table are deprecated.");
+      first_call = FALSE;
+    }
+#endif /* G_ENABLE_DEBUG */
 }
 
 void
 g_scanner_thaw_symbol_table (GScanner *scanner)
 {
-  g_return_if_fail (scanner != NULL);
-  
-  g_hash_table_thaw (scanner->symbol_table);
 }
 
 GTokenType
? test.c
? test
Index: glib.h
===================================================================
RCS file: /cvs/gnome/glib/glib.h,v
retrieving revision 1.165
diff -u -b -B -r1.165 glib.h
--- glib.h	2000/04/26 08:42:19	1.165
+++ glib.h	2000/04/27 11:41:49
@@ -1674,15 +1674,22 @@
 				 gulong	      n,
 				 gchar const *format,
 				 va_list      args);
-gchar*	g_basename		(const gchar *file_name);
 /* Check if a file name is an absolute path */
 gboolean g_path_is_absolute	(const gchar *file_name);
 /* In case of absolute paths, skip the root part */
 gchar*  g_path_skip_root	(gchar       *file_name);
 
-/* strings are newly allocated with g_malloc() */
+/* These two functions are deprecated and will be removed in the next
+ * major release of GLib. Use g_path_get_dirname/g_path_get_basename
+ * instead. Whatch out! The string returned by g_path_get_basename
+ * must be g_freed, while the string returned by g_basename must not.*/
+gchar*	g_basename		(const gchar *file_name);
 gchar*	g_dirname		(const gchar *file_name);
+
+/* The returned strings are newly allocated with g_malloc() */
 gchar*	g_get_current_dir	(void);
+gchar*	g_path_get_basename	(const gchar *file_name);
+gchar*	g_path_get_dirname	(const gchar *file_name);
 
 /* return the environment string for the variable. The returned memory
  * must not be freed. */
Index: gutils.c
===================================================================
RCS file: /cvs/gnome/glib/gutils.c,v
retrieving revision 1.60
diff -u -b -B -r1.60 gutils.c
--- gutils.c	2000/04/19 08:43:52	1.60
+++ gutils.c	2000/04/27 11:41:49
@@ -258,7 +258,18 @@
 g_basename (const gchar	   *file_name)
 {
   register gchar *base;
+#ifdef G_ENABLE_DEBUG
+  static gboolean first_call = TRUE;
   
+  if (first_call)
+    {
+      g_warning("g_basename is deprecated. Use g_path_get_basename instead.");
+      g_warning("Look out! You have to g_free the string returned by "
+		"g_path_get_basename.");
+      first_call = FALSE;
+    }
+#endif /* G_ENABLE_DEBUG */
+  
   g_return_val_if_fail (file_name != NULL, NULL);
   
   base = strrchr (file_name, G_DIR_SEPARATOR);
@@ -273,6 +284,46 @@
   return (gchar*) file_name;
 }
 
+gchar*
+g_path_get_basename (const gchar   *file_name)
+{
+  register gint base;
+  register gint last_nonslash;
+  guint len;
+  gchar *retval;
+ 
+  g_return_val_if_fail (file_name != NULL, NULL);
+  
+  if (file_name[0] == '\0')
+    /* empty string */
+    return g_strdup (".");
+
+  last_nonslash = strlen (file_name) - 1;
+
+  while (last_nonslash >= 0 && file_name [last_nonslash] == G_DIR_SEPARATOR)
+    last_nonslash--;
+
+  if (last_nonslash == -1)
+    /* string only containing slashes */
+    return g_strdup (G_DIR_SEPARATOR_S);
+
+  base = last_nonslash;
+
+  while (base >=0 && file_name [base] != G_DIR_SEPARATOR)
+    base--;
+
+#ifdef G_OS_WIN32
+  if (base == -1 && isalpha (file_name[0]) && file_name[1] == ':')
+    base = 1;
+#endif /* G_OS_WIN32 */
+
+  len = last_nonslash - base;
+  retval = g_malloc (len + 1);
+  memcpy (retval, file_name + base + 1, len);
+  retval [len] = '\0';
+  return retval;
+}
+
 gboolean
 g_path_is_absolute (const gchar *file_name)
 {
@@ -306,7 +357,7 @@
 }
 
 gchar*
-g_dirname (const gchar	   *file_name)
+g_path_get_dirname (const gchar	   *file_name)
 {
   register gchar *base;
   register guint len;
@@ -325,6 +376,22 @@
   base[len] = 0;
   
   return base;
+}
+
+gchar*
+g_dirname (const gchar	   *file_name)
+{
+#ifdef G_ENABLE_DEBUG
+  static gboolean first_call = TRUE;
+
+  if (first_call)
+    {
+      g_warning("g_dirname is deprecated. Use g_path_get_dirname instead.");
+      first_call = FALSE;
+    }
+#endif /* G_ENABLE_DEBUG */
+
+  return g_path_get_dirname (file_name);
 }
 
 gchar*