Add a FIXME to some unclear or wrong code.

2000-10-07  Pavel Cisler <pavel@eazel.com>

	* libnautilus-extensions/nautilus-directory-async.c:
	(dequeue_pending_idle_callback):
	Add a FIXME to some unclear or wrong code.

	* libnautilus-extensions/nautilus-directory.c:
	(nautilus_directory_notify_files_added),
	(nautilus_directory_notify_files_removed),
	(nautilus_directory_notify_files_moved):
	Fix 3050 - Memory trashing during drag&drop.
	Files removed from monitored directories did get an
	unref but they didn't get a ref when added to new
	monitored directories.s
	Add FIXMEs to some unclear, wrong or missing code.

	* libnautilus-extensions/nautilus-file.c:
	(nautilus_file_new_from_name), (nautilus_file_new_from_info),
	(nautilus_file_ref), (nautilus_file_unref):
	Add debugging code to help find ref-counting bugs with
	NautilusFile. Sadly this is not the full ref/unref balance
	debugger I had originally in mind as it seems to be way harder
	to match up refs/unrefs than I thought.

	* libnautilus-extensions/nautilus-file.c:
	(nautilus_file_list_ref), (nautilus_file_list_unref),
	(nautilus_file_list_free), (nautilus_file_list_copy):
	Make these files call nautilus_file_ref/nautilus_file_unref
	instead of gtk_object_ref/gtk_object_unref. We should either
	stick to using nautilus_file_ref/unref or get rid of them.
	Using both pairs of calls is a mess.

7 files changed, 174 insertions, 24 deletions

diff --git a/ChangeLog b/ChangeLog
index 856629e74..b1e5f874b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,35 @@
+2000-10-07  Pavel Cisler <pavel@eazel.com>
+
+	* libnautilus-extensions/nautilus-directory-async.c:
+	(dequeue_pending_idle_callback):
+	Add a FIXME to some unclear or wrong code.
+	
+	* libnautilus-extensions/nautilus-directory.c:
+	(nautilus_directory_notify_files_added),
+	(nautilus_directory_notify_files_removed),
+	(nautilus_directory_notify_files_moved):
+	Fix 3050 - Memory trashing during drag&drop.
+	Files removed from monitored directories did get an
+	unref but they didn't get a ref when added to new
+	monitored directories.s
+	Add FIXMEs to some unclear, wrong or missing code.
+	
+	* libnautilus-extensions/nautilus-file.c:
+	(nautilus_file_new_from_name), (nautilus_file_new_from_info),
+	(nautilus_file_ref), (nautilus_file_unref):
+	Add debugging code to help find ref-counting bugs with
+	NautilusFile. Sadly this is not the full ref/unref balance
+	debugger I had originally in mind as it seems to be way harder 
+	to match up refs/unrefs than I thought.
+
+	* libnautilus-extensions/nautilus-file.c:
+	(nautilus_file_list_ref), (nautilus_file_list_unref),
+	(nautilus_file_list_free), (nautilus_file_list_copy):
+	Make these files call nautilus_file_ref/nautilus_file_unref
+	instead of gtk_object_ref/gtk_object_unref. We should either
+	stick to using nautilus_file_ref/unref or get rid of them. 
+	Using both pairs of calls is a mess.
+	
 2000-10-07  Andy Hertzfeld  <andy@eazel.com>
 
 	* src/nautilus-shell-ui.xml:
diff --git a/libnautilus-extensions/nautilus-directory-async.c b/libnautilus-extensions/nautilus-directory-async.c
index c118e9b40..da7ba8c02 100644
--- a/libnautilus-extensions/nautilus-directory-async.c
+++ b/libnautilus-extensions/nautilus-directory-async.c
@@ -907,6 +907,12 @@ dequeue_pending_idle_callback (gpointer callback_data)
 				g_list_free_1 (p);
 
 				if (!nautilus_directory_is_file_list_monitored (directory)) {
+					/* FIXME: is this right?
+					 * I would expect to have to ref a file if the directory is monitored,
+					 * not the other way around.
+					 * 
+					 * If it's correct, a comment here would help.
+					 */
 					nautilus_file_ref (file);
 				}
 				changed_files = g_list_prepend (changed_files, file);
diff --git a/libnautilus-extensions/nautilus-directory.c b/libnautilus-extensions/nautilus-directory.c
index 44c311288..60b2056c4 100644
--- a/libnautilus-extensions/nautilus-directory.c
+++ b/libnautilus-extensions/nautilus-directory.c
@@ -787,6 +787,7 @@ nautilus_directory_notify_files_added (GList *uris)
 		}
 		hash_table_list_prepend (added_lists, directory, vfs_uri);
 		nautilus_directory_unref (directory);
+		/* FIXME: do we need to ref here if the file is added to a monitored directory? */
 	}
 
 
@@ -839,6 +840,7 @@ nautilus_directory_notify_files_removed (GList *uris)
 		hash_table_list_prepend (changed_lists,
 					 file->details->directory,
 					 file);
+		/* FIXME: do we need to unref here if the file is in a monitored directory? */
 	}
 
 	/* Now send out the changed signals. */
@@ -858,7 +860,7 @@ nautilus_directory_notify_files_moved (GList *uri_pairs)
 	NautilusFile *file;
 	NautilusDirectory *old_directory, *new_directory;
 	GHashTable *parent_directories;
-	GList *new_files_list, *unref_list;
+	GList *new_files_list, *unref_list, *ref_list;
 	GHashTable *added_lists, *changed_lists;
 	GList **files;
 	char *name;
@@ -868,6 +870,7 @@ nautilus_directory_notify_files_moved (GList *uri_pairs)
 	added_lists = g_hash_table_new (g_direct_hash, g_direct_equal);
 	changed_lists = g_hash_table_new (g_direct_hash, g_direct_equal);
 	unref_list = NULL;
+	ref_list = NULL;
 
 	/* Make a list of parent directories that will need their counts updated. */
 	parent_directories = g_hash_table_new (g_direct_hash, g_direct_equal);
@@ -878,7 +881,12 @@ nautilus_directory_notify_files_moved (GList *uri_pairs)
 		/* Move an existing file. */
 		file = nautilus_file_get (pair->from_uri);
 		if (file == NULL) {
-			/* Handle this as it it was a new file. */
+			/* FIXME:
+			 * nautilus_file_get will never return NULL
+			 * what is this really trying to do?
+			 */
+
+			/* Handle this as if it was a new file. */
 			new_files_list = g_list_prepend (new_files_list,
 							 pair->to_uri);
 		} else {
@@ -927,14 +935,21 @@ nautilus_directory_notify_files_moved (GList *uri_pairs)
 				hash_table_list_prepend (added_lists,
 							 new_directory,
 							 file);
-			}
 
-			/* If the old directory was monitoring files, then it
-			 * may have been the only one with a ref to the file.
-			 */
-			if (nautilus_directory_is_file_list_monitored (old_directory)) {
-				unref_list = g_list_prepend (unref_list, file);
+				/* If the old directory was monitoring files, then it
+				 * may have been the only one with a ref to the file.
+				 */
+				if (nautilus_directory_is_file_list_monitored (old_directory)) {
+					unref_list = g_list_prepend (unref_list, file);
+				}
+				/* If the new directory is monitored, need to ref the file
+				 * as it would get if it was in the directory when we started the monitor
+				 */
+				if (nautilus_directory_is_file_list_monitored (new_directory)) {
+					ref_list = g_list_prepend (ref_list, file);
+				}
 			}
+
 			
 			/* Done with the old directory. */
 			nautilus_directory_unref (old_directory);
@@ -950,6 +965,10 @@ nautilus_directory_notify_files_moved (GList *uri_pairs)
 	g_hash_table_foreach (added_lists, call_files_added_free_list, NULL);
 	g_hash_table_destroy (added_lists);
 
+	/* Ref the files we need. */
+	nautilus_file_list_ref (ref_list);
+	g_list_free (ref_list);
+
 	/* Let the file objects go. */
 	nautilus_file_list_free (unref_list);
 
diff --git a/libnautilus-extensions/nautilus-file.c b/libnautilus-extensions/nautilus-file.c
index 878151bf5..b389fbedb 100644
--- a/libnautilus-extensions/nautilus-file.c
+++ b/libnautilus-extensions/nautilus-file.c
@@ -52,6 +52,14 @@
 #include <stdlib.h>
 #include <unistd.h>
 
+#undef NAUTILUS_FILE_DEBUG_REF
+
+#ifdef NAUTILUS_FILE_DEBUG_REF
+extern void eazel_dump_stack_trace	(const char    *print_prefix,
+					 int		num_levels);
+/* from libleakcheck.so */
+#endif
+
 typedef enum {
 	NAUTILUS_DATE_TYPE_MODIFIED,
 	NAUTILUS_DATE_TYPE_CHANGED,
@@ -103,6 +111,7 @@ nautilus_file_initialize (NautilusFile *file)
 	file->details = g_new0 (NautilusFileDetails, 1);
 }
 
+
 static NautilusFile *
 nautilus_file_new_from_name (NautilusDirectory *directory,
 			     const char *name)
@@ -117,6 +126,11 @@ nautilus_file_new_from_name (NautilusDirectory *directory,
 	gtk_object_ref (GTK_OBJECT (file));
 	gtk_object_sink (GTK_OBJECT (file));
 
+#ifdef NAUTILUS_FILE_DEBUG_REF
+	printf("%10p ref'd\n", file);
+	eazel_dump_stack_trace ("\t", 10);
+#endif
+
 	nautilus_directory_ref (directory);
 
 	file->details->directory = directory;
@@ -221,6 +235,12 @@ nautilus_file_new_from_info (NautilusDirectory *directory,
 	g_return_val_if_fail (info != NULL, NULL);
 
 	file = NAUTILUS_FILE (gtk_object_new (NAUTILUS_TYPE_FILE, NULL));
+
+#ifdef NAUTILUS_FILE_DEBUG_REF
+	printf("%10p ref'd\n", file);
+	eazel_dump_stack_trace ("\t", 10);
+#endif
+
 	gtk_object_ref (GTK_OBJECT (file));
 	gtk_object_sink (GTK_OBJECT (file));
 
@@ -401,6 +421,7 @@ destroy (GtkObject *object)
 	NAUTILUS_CALL_PARENT_CLASS (GTK_OBJECT_CLASS, destroy, (object));
 }
 
+
 NautilusFile *
 nautilus_file_ref (NautilusFile *file)
 {
@@ -408,6 +429,12 @@ nautilus_file_ref (NautilusFile *file)
 		return NULL;
 	}
 	g_return_val_if_fail (NAUTILUS_IS_FILE (file), NULL);
+
+#ifdef NAUTILUS_FILE_DEBUG_REF
+	printf("%10p ref'd\n", file);
+	eazel_dump_stack_trace ("\t", 10);
+#endif
+
 	gtk_object_ref (GTK_OBJECT (file));
 	return file;
 }
@@ -421,6 +448,11 @@ nautilus_file_unref (NautilusFile *file)
 
 	g_return_if_fail (NAUTILUS_IS_FILE (file));
 
+#ifdef NAUTILUS_FILE_DEBUG_REF
+	printf("%10p unref'd\n", file);
+	eazel_dump_stack_trace ("\t", 10);
+#endif
+
 	gtk_object_unref (GTK_OBJECT (file));
 }
 
@@ -3933,7 +3965,8 @@ nautilus_file_dump (NautilusFile *file)
 GList *
 nautilus_file_list_ref (GList *list)
 {
-	return nautilus_gtk_object_list_ref (list);
+	g_list_foreach (list, (GFunc) nautilus_file_ref, NULL);
+	return list;
 }
 
 /**
@@ -3945,7 +3978,7 @@ nautilus_file_list_ref (GList *list)
 void
 nautilus_file_list_unref (GList *list)
 {
-	nautilus_gtk_object_list_unref (list);
+	g_list_foreach (list, (GFunc) nautilus_file_unref, NULL);
 }
 
 /**
@@ -3957,7 +3990,8 @@ nautilus_file_list_unref (GList *list)
 void
 nautilus_file_list_free (GList *list)
 {
-	nautilus_gtk_object_list_free (list);
+	nautilus_file_list_unref (list);
+	g_list_free (list);
 }
 
 /**
@@ -3969,7 +4003,7 @@ nautilus_file_list_free (GList *list)
 GList *
 nautilus_file_list_copy (GList *list)
 {
-	return nautilus_gtk_object_list_copy (list);
+	return g_list_copy (nautilus_file_list_ref (list));
 }
 
 /* Extract the top left part of the read-in text. */
diff --git a/libnautilus-private/nautilus-directory-async.c b/libnautilus-private/nautilus-directory-async.c
index c118e9b40..da7ba8c02 100644
--- a/libnautilus-private/nautilus-directory-async.c
+++ b/libnautilus-private/nautilus-directory-async.c
@@ -907,6 +907,12 @@ dequeue_pending_idle_callback (gpointer callback_data)
 				g_list_free_1 (p);
 
 				if (!nautilus_directory_is_file_list_monitored (directory)) {
+					/* FIXME: is this right?
+					 * I would expect to have to ref a file if the directory is monitored,
+					 * not the other way around.
+					 * 
+					 * If it's correct, a comment here would help.
+					 */
 					nautilus_file_ref (file);
 				}
 				changed_files = g_list_prepend (changed_files, file);
diff --git a/libnautilus-private/nautilus-directory.c b/libnautilus-private/nautilus-directory.c
index 44c311288..60b2056c4 100644
--- a/libnautilus-private/nautilus-directory.c
+++ b/libnautilus-private/nautilus-directory.c
@@ -787,6 +787,7 @@ nautilus_directory_notify_files_added (GList *uris)
 		}
 		hash_table_list_prepend (added_lists, directory, vfs_uri);
 		nautilus_directory_unref (directory);
+		/* FIXME: do we need to ref here if the file is added to a monitored directory? */
 	}
 
 
@@ -839,6 +840,7 @@ nautilus_directory_notify_files_removed (GList *uris)
 		hash_table_list_prepend (changed_lists,
 					 file->details->directory,
 					 file);
+		/* FIXME: do we need to unref here if the file is in a monitored directory? */
 	}
 
 	/* Now send out the changed signals. */
@@ -858,7 +860,7 @@ nautilus_directory_notify_files_moved (GList *uri_pairs)
 	NautilusFile *file;
 	NautilusDirectory *old_directory, *new_directory;
 	GHashTable *parent_directories;
-	GList *new_files_list, *unref_list;
+	GList *new_files_list, *unref_list, *ref_list;
 	GHashTable *added_lists, *changed_lists;
 	GList **files;
 	char *name;
@@ -868,6 +870,7 @@ nautilus_directory_notify_files_moved (GList *uri_pairs)
 	added_lists = g_hash_table_new (g_direct_hash, g_direct_equal);
 	changed_lists = g_hash_table_new (g_direct_hash, g_direct_equal);
 	unref_list = NULL;
+	ref_list = NULL;
 
 	/* Make a list of parent directories that will need their counts updated. */
 	parent_directories = g_hash_table_new (g_direct_hash, g_direct_equal);
@@ -878,7 +881,12 @@ nautilus_directory_notify_files_moved (GList *uri_pairs)
 		/* Move an existing file. */
 		file = nautilus_file_get (pair->from_uri);
 		if (file == NULL) {
-			/* Handle this as it it was a new file. */
+			/* FIXME:
+			 * nautilus_file_get will never return NULL
+			 * what is this really trying to do?
+			 */
+
+			/* Handle this as if it was a new file. */
 			new_files_list = g_list_prepend (new_files_list,
 							 pair->to_uri);
 		} else {
@@ -927,14 +935,21 @@ nautilus_directory_notify_files_moved (GList *uri_pairs)
 				hash_table_list_prepend (added_lists,
 							 new_directory,
 							 file);
-			}
 
-			/* If the old directory was monitoring files, then it
-			 * may have been the only one with a ref to the file.
-			 */
-			if (nautilus_directory_is_file_list_monitored (old_directory)) {
-				unref_list = g_list_prepend (unref_list, file);
+				/* If the old directory was monitoring files, then it
+				 * may have been the only one with a ref to the file.
+				 */
+				if (nautilus_directory_is_file_list_monitored (old_directory)) {
+					unref_list = g_list_prepend (unref_list, file);
+				}
+				/* If the new directory is monitored, need to ref the file
+				 * as it would get if it was in the directory when we started the monitor
+				 */
+				if (nautilus_directory_is_file_list_monitored (new_directory)) {
+					ref_list = g_list_prepend (ref_list, file);
+				}
 			}
+
 			
 			/* Done with the old directory. */
 			nautilus_directory_unref (old_directory);
@@ -950,6 +965,10 @@ nautilus_directory_notify_files_moved (GList *uri_pairs)
 	g_hash_table_foreach (added_lists, call_files_added_free_list, NULL);
 	g_hash_table_destroy (added_lists);
 
+	/* Ref the files we need. */
+	nautilus_file_list_ref (ref_list);
+	g_list_free (ref_list);
+
 	/* Let the file objects go. */
 	nautilus_file_list_free (unref_list);
 
diff --git a/libnautilus-private/nautilus-file.c b/libnautilus-private/nautilus-file.c
index 878151bf5..b389fbedb 100644
--- a/libnautilus-private/nautilus-file.c
+++ b/libnautilus-private/nautilus-file.c
@@ -52,6 +52,14 @@
 #include <stdlib.h>
 #include <unistd.h>
 
+#undef NAUTILUS_FILE_DEBUG_REF
+
+#ifdef NAUTILUS_FILE_DEBUG_REF
+extern void eazel_dump_stack_trace	(const char    *print_prefix,
+					 int		num_levels);
+/* from libleakcheck.so */
+#endif
+
 typedef enum {
 	NAUTILUS_DATE_TYPE_MODIFIED,
 	NAUTILUS_DATE_TYPE_CHANGED,
@@ -103,6 +111,7 @@ nautilus_file_initialize (NautilusFile *file)
 	file->details = g_new0 (NautilusFileDetails, 1);
 }
 
+
 static NautilusFile *
 nautilus_file_new_from_name (NautilusDirectory *directory,
 			     const char *name)
@@ -117,6 +126,11 @@ nautilus_file_new_from_name (NautilusDirectory *directory,
 	gtk_object_ref (GTK_OBJECT (file));
 	gtk_object_sink (GTK_OBJECT (file));
 
+#ifdef NAUTILUS_FILE_DEBUG_REF
+	printf("%10p ref'd\n", file);
+	eazel_dump_stack_trace ("\t", 10);
+#endif
+
 	nautilus_directory_ref (directory);
 
 	file->details->directory = directory;
@@ -221,6 +235,12 @@ nautilus_file_new_from_info (NautilusDirectory *directory,
 	g_return_val_if_fail (info != NULL, NULL);
 
 	file = NAUTILUS_FILE (gtk_object_new (NAUTILUS_TYPE_FILE, NULL));
+
+#ifdef NAUTILUS_FILE_DEBUG_REF
+	printf("%10p ref'd\n", file);
+	eazel_dump_stack_trace ("\t", 10);
+#endif
+
 	gtk_object_ref (GTK_OBJECT (file));
 	gtk_object_sink (GTK_OBJECT (file));
 
@@ -401,6 +421,7 @@ destroy (GtkObject *object)
 	NAUTILUS_CALL_PARENT_CLASS (GTK_OBJECT_CLASS, destroy, (object));
 }
 
+
 NautilusFile *
 nautilus_file_ref (NautilusFile *file)
 {
@@ -408,6 +429,12 @@ nautilus_file_ref (NautilusFile *file)
 		return NULL;
 	}
 	g_return_val_if_fail (NAUTILUS_IS_FILE (file), NULL);
+
+#ifdef NAUTILUS_FILE_DEBUG_REF
+	printf("%10p ref'd\n", file);
+	eazel_dump_stack_trace ("\t", 10);
+#endif
+
 	gtk_object_ref (GTK_OBJECT (file));
 	return file;
 }
@@ -421,6 +448,11 @@ nautilus_file_unref (NautilusFile *file)
 
 	g_return_if_fail (NAUTILUS_IS_FILE (file));
 
+#ifdef NAUTILUS_FILE_DEBUG_REF
+	printf("%10p unref'd\n", file);
+	eazel_dump_stack_trace ("\t", 10);
+#endif
+
 	gtk_object_unref (GTK_OBJECT (file));
 }
 
@@ -3933,7 +3965,8 @@ nautilus_file_dump (NautilusFile *file)
 GList *
 nautilus_file_list_ref (GList *list)
 {
-	return nautilus_gtk_object_list_ref (list);
+	g_list_foreach (list, (GFunc) nautilus_file_ref, NULL);
+	return list;
 }
 
 /**
@@ -3945,7 +3978,7 @@ nautilus_file_list_ref (GList *list)
 void
 nautilus_file_list_unref (GList *list)
 {
-	nautilus_gtk_object_list_unref (list);
+	g_list_foreach (list, (GFunc) nautilus_file_unref, NULL);
 }
 
 /**
@@ -3957,7 +3990,8 @@ nautilus_file_list_unref (GList *list)
 void
 nautilus_file_list_free (GList *list)
 {
-	nautilus_gtk_object_list_free (list);
+	nautilus_file_list_unref (list);
+	g_list_free (list);
 }
 
 /**
@@ -3969,7 +4003,7 @@ nautilus_file_list_free (GList *list)
 GList *
 nautilus_file_list_copy (GList *list)
 {
-	return nautilus_gtk_object_list_copy (list);
+	return g_list_copy (nautilus_file_list_ref (list));
 }
 
 /* Extract the top left part of the read-in text. */