directory: simplify constructors and clarify API documentation

src/db/simple_db_plugin.c

@@ -199,7 +199,7 @@ simple_db_open(struct db *_db, G_GNUC_UNUSED GError **error_r)
 {
 	struct simple_db *db = (struct simple_db *)_db;
 
-	db->root = directory_new("", NULL);
+	db->root = directory_new_root();
 	db->mtime = 0;
 
 	GError *error = NULL;
@@ -212,7 +212,7 @@ simple_db_open(struct db *_db, G_GNUC_UNUSED GError **error_r)
 		if (!simple_db_check(db, error_r))
 			return false;
 
-		db->root = directory_new("", NULL);
+		db->root = directory_new_root();
 	}
 
 	return true;

src/directory.c

@@ -66,12 +66,47 @@ directory_free(struct directory *directory)
 	/*directory_get_path(NULL); */
 }
 
+void
+directory_delete(struct directory *directory)
+{
+	assert(directory != NULL);
+	assert(directory->parent != NULL);
+
+	dirvec_delete(&directory->parent->children, directory);
+	directory_free(directory);
+}
+
 const char *
 directory_get_name(const struct directory *directory)
 {
 	return g_basename(directory->path);
 }
 
+struct directory *
+directory_new_child(struct directory *parent, const char *name_utf8)
+{
+	assert(parent != NULL);
+	assert(name_utf8 != NULL);
+	assert(*name_utf8 != 0);
+
+	char *allocated;
+	const char *path_utf8;
+	if (directory_is_root(parent)) {
+		allocated = NULL;
+		path_utf8 = name_utf8;
+	} else {
+		allocated = g_strconcat(directory_get_path(parent),
+					"/", name_utf8, NULL);
+		path_utf8 = allocated;
+	}
+
+	struct directory *directory = directory_new(path_utf8, parent);
+	g_free(allocated);
+
+	dirvec_add(&parent->children, directory);
+	return directory;
+}
+
 void
 directory_prune_empty(struct directory *directory)
 {
@@ -83,10 +118,8 @@ directory_prune_empty(struct directory *directory)
 
 		directory_prune_empty(child);
 
-		if (directory_is_empty(child)) {
-			dirvec_delete(dv, child);
-			directory_free(child);
-		}
+		if (directory_is_empty(child))
+			directory_delete(child);
 	}
 	if (!dv->nr)
 		dirvec_destroy(dv);

src/directory.h

@@ -56,12 +56,37 @@ isRootDirectory(const char *name)
 	return name[0] == 0 || (name[0] == '/' && name[1] == 0);
 }
 
+/**
+ * Generic constructor for #directory object.
+ */
+G_GNUC_MALLOC
 struct directory *
 directory_new(const char *dirname, struct directory *parent);
 
+/**
+ * Create a new root #directory object.
+ */
+G_GNUC_MALLOC
+static inline struct directory *
+directory_new_root(void)
+{
+	return directory_new("", NULL);
+}
+
+/**
+ * Free this #directory object (and the whole object tree within it),
+ * assuming it was already removed from the parent.
+ */
 void
 directory_free(struct directory *directory);
 
+/**
+ * Remove this #directory object from its parent and free it.  This
+ * must not be called with the root directory.
+ */
+void
+directory_delete(struct directory *directory);
+
 static inline bool
 directory_is_empty(const struct directory *directory)
 {
@@ -87,6 +112,7 @@ directory_is_root(const struct directory *directory)
 /**
  * Returns the base name of the directory.
  */
+G_GNUC_PURE
 const char *
 directory_get_name(const struct directory *directory);
 
@@ -96,12 +122,27 @@ directory_get_child(const struct directory *directory, const char *name)
 	return dirvec_find(&directory->children, name);
 }
 
+/**
+ * Create a new #directory object as a child of the given one.
+ *
+ * @param parent the parent directory the new one will be added to
+ * @param name_utf8 the UTF-8 encoded name of the new sub directory
+ */
+G_GNUC_MALLOC
+struct directory *
+directory_new_child(struct directory *parent, const char *name_utf8);
+
+/**
+ * Look up a sub directory, and create the object if it does not
+ * exist.
+ */
 static inline struct directory *
-directory_new_child(struct directory *directory, const char *name)
+directory_make_child(struct directory *directory, const char *name_utf8)
 {
-	struct directory *subdir = directory_new(name, directory);
-	dirvec_add(&directory->children, subdir);
-	return subdir;
+	struct directory *child = directory_get_child(directory, name_utf8);
+	if (child == NULL)
+		child = directory_new_child(directory, name_utf8);
+	return child;
 }
 
 void

src/directory_save.c

@@ -81,7 +81,6 @@ static struct directory *
 directory_load_subdir(FILE *fp, struct directory *parent, const char *name,
 		      GString *buffer, GError **error_r)
 {
-	struct directory *directory;
 	const char *line;
 	bool success;
 
@@ -91,20 +90,13 @@ directory_load_subdir(FILE *fp, struct directory *parent, const char *name,
 		return NULL;
 	}
 
-	if (directory_is_root(parent)) {
-		directory = directory_new(name, parent);
-	} else {
-		char *path = g_strconcat(directory_get_path(parent), "/",
-					 name, NULL);
-		directory = directory_new(path, parent);
-		g_free(path);
-	}
+	struct directory *directory = directory_new_child(parent, name);
 
 	line = read_text_line(fp, buffer);
 	if (line == NULL) {
 		g_set_error(error_r, directory_quark(), 0,
 			    "Unexpected end of file");
-		directory_free(directory);
+		directory_delete(directory);
 		return NULL;
 	}
 
@@ -117,7 +109,7 @@ directory_load_subdir(FILE *fp, struct directory *parent, const char *name,
 		if (line == NULL) {
 			g_set_error(error_r, directory_quark(), 0,
 				    "Unexpected end of file");
-			directory_free(directory);
+			directory_delete(directory);
 			return NULL;
 		}
 	}
@@ -125,13 +117,13 @@ directory_load_subdir(FILE *fp, struct directory *parent, const char *name,
 	if (!g_str_has_prefix(line, DIRECTORY_BEGIN)) {
 		g_set_error(error_r, directory_quark(), 0,
 			    "Malformed line: %s", line);
-		directory_free(directory);
+		directory_delete(directory);
 		return NULL;
 	}
 
 	success = directory_load(fp, directory, buffer, error_r);
 	if (!success) {
-		directory_free(directory);
+		directory_delete(directory);
 		return NULL;
 	}
 
@@ -153,8 +145,6 @@ directory_load(FILE *fp, struct directory *directory,
 						      buffer, error);
 			if (subdir == NULL)
 				return false;
-
-			dirvec_add(&directory->children, subdir);
 		} else if (g_str_has_prefix(line, SONG_BEGIN)) {
 			const char *name = line + sizeof(SONG_BEGIN) - 1;
 			struct song *song;

src/update_walk.c

@@ -140,9 +140,7 @@ delete_directory(struct directory *directory)
 	assert(directory->parent != NULL);
 
 	clear_directory(directory);
-
-	dirvec_delete(&directory->parent->children, directory);
-	directory_free(directory);
+	directory_delete(directory);
 }
 
 static void
@@ -272,7 +270,6 @@ removeDeletedFromDirectory(struct directory *directory)
 		if (directory_exists(dv->base[i]))
 			continue;
 
-		g_debug("removing directory: %s", dv->base[i]->path);
 		delete_directory(dv->base[i]);
 		modified = true;
 	}
@@ -363,28 +360,6 @@ inodeFoundInParent(struct directory *parent, ino_t inode, dev_t device)
 	return 0;
 }
 
-static struct directory *
-make_subdir(struct directory *parent, const char *name)
-{
-	struct directory *directory;
-
-	directory = directory_get_child(parent, name);
-	if (directory == NULL) {
-		char *path;
-
-		if (directory_is_root(parent))
-			path = NULL;
-		else
-			name = path = g_strconcat(directory_get_path(parent),
-						  "/", name, NULL);
-
-		directory = directory_new_child(parent, name);
-		g_free(path);
-	}
-
-	return directory;
-}
-
 #ifdef ENABLE_ARCHIVE
 static void
 update_archive_tree(struct directory *directory, char *name)
@@ -397,9 +372,9 @@ update_archive_tree(struct directory *directory, char *name)
 	if (tmp) {
 		*tmp = 0;
 		//add dir is not there already
-		if ((subdir = dirvec_find(&directory->children, name)) == NULL) {
+		if ((subdir = directory_get_child(directory, name)) == NULL) {
 		        //create new directory
-		        subdir = make_subdir(directory, name);
+			subdir = directory_new_child(directory, name);
 			subdir->device = DEVICE_INARCHIVE;
 		}
 		//create directories first
@@ -442,7 +417,7 @@ update_archive_file(struct directory *parent, const char *name,
 	struct directory *directory;
 	char *filepath;
 
-	directory = dirvec_find(&parent->children, name);
+	directory = directory_get_child(parent, name);
 	if (directory != NULL && directory->mtime == st->st_mtime &&
 	    !walk_discard)
 		/* MPD has already scanned the archive, and it hasn't
@@ -465,7 +440,7 @@ update_archive_file(struct directory *parent, const char *name,
 
 	if (directory == NULL) {
 		g_debug("creating archive directory: %s", name);
-		directory = make_subdir(parent, name);
+		directory = directory_new_child(parent, name);
 		/* mark this directory as archive (we use device for
 		   this) */
 		directory->device = DEVICE_INARCHIVE;
@@ -494,7 +469,7 @@ update_container_file(	struct directory* directory,
 	char* vtrack = NULL;
 	unsigned int tnum = 0;
 	char* pathname = map_directory_child_fs(directory, name);
-	struct directory* contdir = dirvec_find(&directory->children, name);
+	struct directory *contdir = directory_get_child(directory, name);
 
 	// directory exists already
 	if (contdir != NULL)
@@ -515,7 +490,7 @@ update_container_file(	struct directory* directory,
 		}
 	}
 
-	contdir = make_subdir(directory, name);
+	contdir = directory_make_child(directory, name);
 	contdir->mtime = st->st_mtime;
 	contdir->device = DEVICE_CONTAINER;
 
@@ -670,7 +645,7 @@ updateInDirectory(struct directory *directory,
 		if (inodeFoundInParent(directory, st->st_ino, st->st_dev))
 			return;
 
-		subdir = make_subdir(directory, name);
+		subdir = directory_make_child(directory, name);
 		assert(directory == subdir->parent);
 
 		ret = updateDirectory(subdir, st);
@@ -829,34 +804,27 @@ updateDirectory(struct directory *directory, const struct stat *st)
 }
 
 static struct directory *
-directory_make_child_checked(struct directory *parent, const char *path)
+directory_make_child_checked(struct directory *parent, const char *name_utf8)
 {
 	struct directory *directory;
-	char *base;
 	struct stat st;
 	struct song *conflicting;
 
-	directory = directory_get_child(parent, path);
+	directory = directory_get_child(parent, name_utf8);
 	if (directory != NULL)
 		return directory;
 
-	base = g_path_get_basename(path);
-
-	if (stat_directory_child(parent, base, &st) < 0 ||
-	    inodeFoundInParent(parent, st.st_ino, st.st_dev)) {
-		g_free(base);
+	if (stat_directory_child(parent, name_utf8, &st) < 0 ||
+	    inodeFoundInParent(parent, st.st_ino, st.st_dev))
 		return NULL;
-	}
 
 	/* if we're adding directory paths, make sure to delete filenames
 	   with potentially the same name */
-	conflicting = songvec_find(&parent->songs, base);
+	conflicting = songvec_find(&parent->songs, name_utf8);
 	if (conflicting)
 		delete_song(parent, conflicting);
 
-	g_free(base);
-
-	directory = directory_new_child(parent, path);
+	directory = directory_new_child(parent, name_utf8);
 	directory_set_stat(directory, &st);
 	return directory;
 }
@@ -866,17 +834,20 @@ addParentPathToDB(const char *utf8path)
 {
 	struct directory *directory = db_get_root();
 	char *duplicated = g_strdup(utf8path);
-	char *slash = duplicated;
+	char *name_utf8 = duplicated, *slash;
 
-	while ((slash = strchr(slash, '/')) != NULL) {
+	while ((slash = strchr(name_utf8, '/')) != NULL) {
 		*slash = 0;
 
+		if (*name_utf8 == 0)
+			continue;
+
 		directory = directory_make_child_checked(directory,
-							 duplicated);
-		if (directory == NULL || slash == NULL)
+							 name_utf8);
+		if (directory == NULL)
 			break;
 
-		*slash++ = '/';
+		name_utf8 = slash + 1;
 	}
 
 	g_free(duplicated);