Fixes a double-ref-counting bug in file_data_new_simple and `file_d…

…ata_new_group`

The bugs would cause all FileData objects created by `file_data_new_simple` or
`file_data_new_group` to start off with a ref-count of 2, even though the only
one reference to it had been created (the one returned by each method).

As a result, creating the FileData and then immediately calling `file_data_unref`
would not free it as would be expected.

Also zero-initializes `struct stat st` so that its contents will be valid even if `stat_utf8` fails.

src/filedata/filedata.cc

@@ -496,7 +496,7 @@ FileData *FileData::file_data_new_local(const gchar *path, struct stat *st, gboo
 
 FileData *FileData::file_data_new_simple(const gchar *path_utf8, FileDataContext *context)
 {
-	struct stat st;
+	struct stat st{};
 
 	if (!stat_utf8(path_utf8, &st))
 		{
@@ -510,11 +510,11 @@ FileData *FileData::file_data_new_simple(const gchar *path_utf8, FileDataContext
 		}
 
 	auto *fd = static_cast<FileData *>(g_hash_table_lookup(context->file_data_pool, path_utf8));
-	// TODO(xsdg): This looks like it double-refs fd if it needed to be
-	// created.  Figure out what should happen here and either fix or
-	// document.
-	if (!fd) fd = file_data_new(path_utf8, &st, TRUE, context);
-	if (fd)
+	if (!fd)
+		{
+		fd = file_data_new(path_utf8, &st, TRUE, context);
+		}
+	else
 		{
 		::file_data_ref(fd);
 		}
@@ -1211,16 +1211,12 @@ void FileData::file_data_basename_hash_to_sidecars(gpointer, gpointer value, gpo
 
 FileData *FileData::file_data_new_group(const gchar *path_utf8, FileDataContext *context)
 {
-	gchar *dir;
-	struct stat st;
-	FileData *fd;
-	GList *files;
-
 	if (context == nullptr)
 		{
 		context = FileData::DefaultFileDataContext();
 		}
 
+	struct stat st{};
 	if (!stat_utf8(path_utf8, &st))
 		{
 		st.st_size = 0;
@@ -1230,13 +1226,17 @@ FileData *FileData::file_data_new_group(const gchar *path_utf8, FileDataContext
 	if (S_ISDIR(st.st_mode))
 		return file_data_new(path_utf8, &st, TRUE, context);
 
-	dir = remove_level_from_path(path_utf8);
+	gchar *dir = remove_level_from_path(path_utf8);
 
+        GList *files;
 	FileList::read_list_real(dir, &files, nullptr, TRUE);
 
-	fd = static_cast<FileData *>(g_hash_table_lookup(context->file_data_pool, path_utf8));
-	if (!fd) fd = file_data_new(path_utf8, &st, TRUE, context);
-	if (fd)
+	auto *fd = static_cast<FileData *>(g_hash_table_lookup(context->file_data_pool, path_utf8));
+	if (!fd)
+		{
+		fd = file_data_new(path_utf8, &st, TRUE, context);
+		}
+	else
 		{
 		::file_data_ref(fd);
 		}

src/tests/filedata/filedata.cc

@@ -124,7 +124,6 @@ TEST_F(FileDataTest, FileDataNewSimpleAndFree)
 	auto *_fd = FileData::file_data_new_simple("/does/not/exist.jpg", &context);
 	ASSERT_NE(_fd, nullptr);
 	ASSERT_EQ(1, context.global_file_data_count);
-	// This currently fails because of a bug in file_data_new_simple.
 	ASSERT_EQ(1, _fd->ref);
 
 	_fd->file_data_unref();
@@ -139,7 +138,6 @@ TEST_F(FileDataTest, FileDataNewGroupAndFree)
 	auto *_fd = FileData::file_data_new_group("/does/not/exist/file.jpg", &context);
 	ASSERT_NE(_fd, nullptr);
 	EXPECT_EQ(1, context.global_file_data_count);
-	// This currently fails because of a bug in file_data_new_simple.
 	ASSERT_EQ(1, _fd->ref);
 
 	_fd->file_data_unref();