Fix a memory leak on image loading

In particular, this handles screenshots from Rigol oscilloscopes,
which reuse the same name series with the same file modification time.

fiv-browser.c

@@ -105,14 +105,11 @@ struct _FivBrowser {
 
 /// The "last modified" timestamp of source images for thumbnails.
 static cairo_user_data_key_t fiv_browser_key_mtime_msec;
+/// The original file size of source images for thumbnails.
+static cairo_user_data_key_t fiv_browser_key_filesize;
 
-// TODO(p): Include FivIoModelEntry data by reference.
 struct entry {
-	gchar *uri;                         ///< GIO URI
-	gchar *target_uri;                  ///< GIO URI for any target
-	gchar *display_name;                ///< Label for the file
-	gint64 mtime_msec;                  ///< Modification time in milliseconds
-
+	FivIoModelEntry *e;                 ///< Reference to model entry
 	cairo_surface_t *thumbnail;         ///< Prescaled thumbnail
 	GIcon *icon;                        ///< If no thumbnail, use this icon
 };
@@ -120,9 +117,7 @@ struct entry {
 static void
 entry_free(Entry *self)
 {
-	g_free(self->uri);
-	g_free(self->target_uri);
-	g_free(self->display_name);
+	fiv_io_model_entry_unref(self->e);
 	g_clear_pointer(&self->thumbnail, cairo_surface_destroy);
 	g_clear_object(&self->icon);
 }
@@ -200,9 +195,6 @@ relayout(FivBrowser *self, int width)
 	gtk_style_context_get_padding(style, GTK_STATE_FLAG_NORMAL, &padding);
 	int available_width = width - padding.left - padding.right, max_width = 0;
 
-	// TODO(p): Remember the first visible item and the vertical offset into it,
-	// then try to ensure its visibility at the end (useful for reloads).
-
 	g_array_set_size(self->layouted_rows, 0);
 	// Whatever self->drag_begin_* used to point at might no longer be there,
 	// but thumbnail reloading would disrupt mouse clicks if we cleared them.
@@ -228,7 +220,8 @@ relayout(FivBrowser *self, int width)
 
 		PangoLayout *label = NULL;
 		if (self->show_labels) {
-			label = gtk_widget_create_pango_layout(widget, entry->display_name);
+			label = gtk_widget_create_pango_layout(
+				widget, entry->e->display_name);
 			pango_layout_set_width(
 				label, (width - 2 * self->glow_w) * PANGO_SCALE);
 			pango_layout_set_alignment(label, PANGO_ALIGN_CENTER);
@@ -505,15 +498,28 @@ rescale_thumbnail(cairo_surface_t *thumbnail, double row_height)
 	return scaled;
 }
 
-static char *
+static const char *
 entry_system_wide_uri(const Entry *self)
 {
 	// "recent" and "trash", e.g., also have "standard::target-uri" set,
 	// but we'd like to avoid saving their thumbnails.
-	if (self->target_uri && fiv_collection_uri_matches(self->uri))
-		return self->target_uri;
+	if (self->e->target_uri && fiv_collection_uri_matches(self->e->uri))
+		return self->e->target_uri;
 
-	return self->uri;
+	return self->e->uri;
+}
+
+static void
+entry_set_surface_user_data(const Entry *self)
+{
+	// This choice of mtime favours unnecessary thumbnail reloading
+	// over retaining stale data (consider both calling functions).
+	cairo_surface_set_user_data(self->thumbnail,
+		&fiv_browser_key_mtime_msec, (void *) (intptr_t) self->e->mtime_msec,
+		NULL);
+	cairo_surface_set_user_data(self->thumbnail,
+		&fiv_browser_key_filesize, (void *) (uintptr_t) self->e->filesize,
+		NULL);
 }
 
 static void
@@ -525,31 +531,32 @@ entry_add_thumbnail(gpointer data, gpointer user_data)
 
 	FivBrowser *browser = FIV_BROWSER(user_data);
 	cairo_surface_t *cached =
-		g_hash_table_lookup(browser->thumbnail_cache, self->uri);
+		g_hash_table_lookup(browser->thumbnail_cache, self->e->uri);
 	if (cached &&
-		(intptr_t) cairo_surface_get_user_data(
-			cached, &fiv_browser_key_mtime_msec) == self->mtime_msec) {
+		(intptr_t) cairo_surface_get_user_data(cached,
+			&fiv_browser_key_mtime_msec) == (intptr_t) self->e->mtime_msec &&
+		(uintptr_t) cairo_surface_get_user_data(cached,
+			&fiv_browser_key_filesize) == (uintptr_t) self->e->filesize) {
 		self->thumbnail = cairo_surface_reference(cached);
 		// TODO(p): If this hit is low-quality, see if a high-quality thumbnail
 		// hasn't been produced without our knowledge (avoid launching a minion
 		// unnecessarily; we might also shift the concern there).
 	} else {
 		cairo_surface_t *found = fiv_thumbnail_lookup(
-			entry_system_wide_uri(self), self->mtime_msec, browser->item_size);
+			entry_system_wide_uri(self), self->e->mtime_msec, self->e->filesize,
+			browser->item_size);
 		self->thumbnail = rescale_thumbnail(found, browser->item_height);
 	}
 
 	if (self->thumbnail) {
-		// This choice of mtime favours unnecessary thumbnail reloading.
-		cairo_surface_set_user_data(self->thumbnail,
-			&fiv_browser_key_mtime_msec, (void *) (intptr_t) self->mtime_msec,
-			NULL);
+		// Yes, this is a pointless action in case it's been found in the cache.
+		entry_set_surface_user_data(self);
 		return;
 	}
 
 	// Fall back to symbolic icons, though there's only so much we can do
 	// in parallel--GTK+ isn't thread-safe.
-	GFile *file = g_file_new_for_uri(self->uri);
+	GFile *file = g_file_new_for_uri(self->e->uri);
 	GFileInfo *info = g_file_query_info(file,
 		G_FILE_ATTRIBUTE_STANDARD_NAME
 		"," G_FILE_ATTRIBUTE_STANDARD_SYMBOLIC_ICON,
@@ -626,7 +633,7 @@ reload_thumbnails(FivBrowser *self)
 	for (guint i = 0; i < self->entries->len; i++) {
 		Entry *entry = &g_array_index(self->entries, Entry, i);
 		if (entry->thumbnail) {
-			g_hash_table_insert(self->thumbnail_cache, g_strdup(entry->uri),
+			g_hash_table_insert(self->thumbnail_cache, g_strdup(entry->e->uri),
 				cairo_surface_reference(entry->thumbnail));
 		}
 
@@ -665,12 +672,8 @@ thumbnailer_reprocess_entry(FivBrowser *self, GBytes *output, Entry *entry)
 		g_queue_push_tail(&self->thumbnailers_queue, entry);
 	}
 
-	// This choice of mtime favours unnecessary thumbnail reloading
-	// over retaining stale data.
-	cairo_surface_set_user_data(entry->thumbnail,
-		&fiv_browser_key_mtime_msec, (void *) (intptr_t) entry->mtime_msec,
-		NULL);
-	g_hash_table_insert(self->thumbnail_cache, g_strdup(entry->uri),
+	entry_set_surface_user_data(entry);
+	g_hash_table_insert(self->thumbnail_cache, g_strdup(entry->e->uri),
 		cairo_surface_reference(entry->thumbnail));
 }
 
@@ -1118,7 +1121,7 @@ fiv_browser_draw(GtkWidget *widget, cairo_t *cr)
 static gboolean
 open_entry(GtkWidget *self, const Entry *entry, gboolean new_window)
 {
-	GFile *location = g_file_new_for_uri(entry->uri);
+	GFile *location = g_file_new_for_uri(entry->e->uri);
 	g_signal_emit(self, browser_signals[ITEM_ACTIVATED], 0, location,
 		new_window ? GTK_PLACES_OPEN_NEW_WINDOW : GTK_PLACES_OPEN_NORMAL);
 	g_object_unref(location);
@@ -1188,7 +1191,7 @@ fiv_browser_button_press_event(GtkWidget *widget, GdkEventButton *event)
 		// no matter what its new location is.
 		gdk_window_set_cursor(gtk_widget_get_window(widget), NULL);
 
-		GFile *file = g_file_new_for_uri(entry->uri);
+		GFile *file = g_file_new_for_uri(entry->e->uri);
 		show_context_menu(widget, file);
 		g_object_unref(file);
 		return GDK_EVENT_STOP;
@@ -1332,8 +1335,8 @@ fiv_browser_drag_data_get(GtkWidget *widget,
 {
 	FivBrowser *self = FIV_BROWSER(widget);
 	if (self->selected) {
-		(void) gtk_selection_data_set_uris(
-			data, (gchar *[]) {entry_system_wide_uri(self->selected), NULL});
+		(void) gtk_selection_data_set_uris(data, (gchar *[])
+			{(gchar *) entry_system_wide_uri(self->selected), NULL});
 	}
 }
 
@@ -1510,7 +1513,7 @@ fiv_browser_key_press_event(GtkWidget *widget, GdkEventKey *event)
 		case GDK_KEY_Return:
 			if (self->selected) {
 				GtkWindow *window = GTK_WINDOW(gtk_widget_get_toplevel(widget));
-				fiv_context_menu_information(window, self->selected->uri);
+				fiv_context_menu_information(window, self->selected->e->uri);
 			}
 			return GDK_EVENT_STOP;
 		}
@@ -1545,7 +1548,7 @@ fiv_browser_query_tooltip(GtkWidget *widget, gint x, gint y,
 	if (!entry)
 		return FALSE;
 
-	gtk_tooltip_set_text(tooltip, entry->display_name);
+	gtk_tooltip_set_text(tooltip, entry->e->display_name);
 	return TRUE;
 }
 
@@ -1559,7 +1562,7 @@ fiv_browser_popup_menu(GtkWidget *widget)
 	GFile *file = NULL;
 	GdkRectangle rect = {};
 	if (self->selected) {
-		file = g_file_new_for_uri(self->selected->uri);
+		file = g_file_new_for_uri(self->selected->e->uri);
 		rect = entry_rect(self, self->selected);
 		rect.x += rect.width / 2;
 		rect.y += rect.height / 2;
@@ -1597,7 +1600,7 @@ on_long_press(GtkGestureLongPress *lp, gdouble x, gdouble y, gpointer user_data)
 
 	// It might also be possible to have long-press just select items,
 	// and show some kind of toolbar with available actions.
-	GFile *file = g_file_new_for_uri(entry->uri);
+	GFile *file = g_file_new_for_uri(entry->e->uri);
 	gtk_menu_popup_at_rect(fiv_context_menu_new(widget, file), window,
 		&(GdkRectangle) {.x = x, .y = y}, GDK_GRAVITY_NORTH_WEST,
 		GDK_GRAVITY_NORTH_WEST, event);
@@ -1813,20 +1816,16 @@ on_model_files_changed(FivIoModel *model, FivBrowser *self)
 
 	gchar *selected_uri = NULL;
 	if (self->selected)
-		selected_uri = g_strdup(self->selected->uri);
+		selected_uri = g_strdup(self->selected->e->uri);
 
 	thumbnailers_abort(self);
 	g_array_set_size(self->entries, 0);
 	g_array_set_size(self->layouted_rows, 0);
 
 	gsize len = 0;
-	const FivIoModelEntry *files = fiv_io_model_get_files(self->model, &len);
+	FivIoModelEntry *const *files = fiv_io_model_get_files(self->model, &len);
 	for (gsize i = 0; i < len; i++) {
-		Entry e = {.thumbnail = NULL,
-			.uri = g_strdup(files[i].uri),
-			.target_uri = g_strdup(files[i].target_uri),
-			.display_name = g_strdup(files[i].display_name),
-			.mtime_msec = files[i].mtime_msec};
+		Entry e = {.e = fiv_io_model_entry_ref(files[i])};
 		g_array_append_val(self->entries, e);
 	}
 
@@ -1877,7 +1876,7 @@ fiv_browser_select(FivBrowser *self, const char *uri)
 
 	for (guint i = 0; i < self->entries->len; i++) {
 		const Entry *entry = &g_array_index(self->entries, Entry, i);
-		if (!g_strcmp0(entry->uri, uri)) {
+		if (!g_strcmp0(entry->e->uri, uri)) {
 			self->selected = entry;
 			scroll_to_selection(self);
 			break;

fiv-io.c

@@ -2754,7 +2754,10 @@ fiv_io_open(const FivIoOpenContext *ctx, GError **error)
 
 	gchar *data = NULL;
 	gsize len = 0;
-	if (!g_file_load_contents(file, NULL, &data, &len, NULL, error))
+	gboolean success =
+		g_file_load_contents(file, NULL, &data, &len, NULL, error);
+	g_object_unref(file);
+	if (!success)
 		return NULL;
 
 	cairo_surface_t *surface = fiv_io_open_from_data(data, len, ctx, error);
@@ -3044,21 +3047,10 @@ fiv_io_serialize_for_search(cairo_surface_t *surface, GError **error)
 
 #include "xdg.h"
 
-static void
-model_entry_finalize(FivIoModelEntry *entry)
-{
-	g_free(entry->uri);
-	g_free(entry->target_uri);
-	g_free(entry->display_name);
-	g_free(entry->collate_key);
-}
-
-static GArray *
+static GPtrArray *
 model_entry_array_new(void)
 {
-	GArray *a = g_array_new(FALSE, TRUE, sizeof(FivIoModelEntry));
-	g_array_set_clear_func(a, (GDestroyNotify) model_entry_finalize);
-	return a;
+	return g_ptr_array_new_with_free_func(g_rc_box_release);
 }
 
 struct _FivIoModel {
@@ -3067,8 +3059,8 @@ struct _FivIoModel {
 
 	GFile *directory;                   ///< Currently loaded directory
 	GFileMonitor *monitor;              ///< "directory" monitoring
-	GArray *subdirs;                    ///< "directory" contents
-	GArray *files;                      ///< "directory" contents
+	GPtrArray *subdirs;                 ///< "directory" contents
+	GPtrArray *files;                   ///< "directory" contents
 
 	FivIoModelSort sort_field;          ///< How to sort
 	gboolean sort_descending;           ///< Whether to sort in reverse
@@ -3150,8 +3142,8 @@ model_compare_entries(FivIoModel *self,
 static gint
 model_compare(gconstpointer a, gconstpointer b, gpointer user_data)
 {
-	const FivIoModelEntry *entry1 = a;
-	const FivIoModelEntry *entry2 = b;
+	const FivIoModelEntry *entry1 = *(const FivIoModelEntry **) a;
+	const FivIoModelEntry *entry2 = *(const FivIoModelEntry **) b;
 	GFile *file1 = g_file_new_for_uri(entry1->uri);
 	GFile *file2 = g_file_new_for_uri(entry2->uri);
 	int result = model_compare_entries(user_data, entry1, file1, entry2, file2);
@@ -3160,18 +3152,86 @@ model_compare(gconstpointer a, gconstpointer b, gpointer user_data)
 	return result;
 }
 
+static size_t
+model_strsize(const char *string)
+{
+	if (!string)
+		return 0;
+
+	return strlen(string) + 1;
+}
+
+static char *
+model_strappend(char **p, const char *string, size_t size)
+{
+	if (!string)
+		return *p;
+
+	char *destination = memcpy(*p, string, size);
+	*p += size;
+	return destination;
+}
+
+static FivIoModelEntry *
+model_entry_new(GFile *file, GFileInfo *info)
+{
+	gchar *uri = g_file_get_uri(file);
+	const gchar *target_uri = g_file_info_get_attribute_string(
+		info, G_FILE_ATTRIBUTE_STANDARD_TARGET_URI);
+	const gchar *display_name = g_file_info_get_display_name(info);
+
+	// TODO(p): Make it possible to use g_utf8_collate_key() instead,
+	// which does not use natural sorting.
+	gchar *parse_name = g_file_get_parse_name(file);
+	gchar *collate_key = g_utf8_collate_key_for_filename(parse_name, -1);
+	g_free(parse_name);
+
+	// The entries are immutable. Packing them into the structure
+	// should help memory usage as well as performance.
+	size_t size_uri          = model_strsize(uri);
+	size_t size_target_uri   = model_strsize(target_uri);
+	size_t size_display_name = model_strsize(display_name);
+	size_t size_collate_key  = model_strsize(collate_key);
+
+	FivIoModelEntry *entry = g_rc_box_alloc0(sizeof *entry +
+		size_uri +
+		size_target_uri +
+		size_display_name +
+		size_collate_key);
+
+	gchar *p = (gchar *) entry + sizeof *entry;
+	entry->uri          = model_strappend(&p, uri, size_uri);
+	entry->target_uri   = model_strappend(&p, target_uri, size_target_uri);
+	entry->display_name = model_strappend(&p, display_name, size_display_name);
+	entry->collate_key  = model_strappend(&p, collate_key, size_collate_key);
+
+	entry->filesize = (guint64) g_file_info_get_size(info);
+
+	GDateTime *mtime = g_file_info_get_modification_date_time(info);
+	if (mtime) {
+		entry->mtime_msec = g_date_time_to_unix(mtime) * 1000 +
+			g_date_time_get_microsecond(mtime) / 1000;
+		g_date_time_unref(mtime);
+	}
+
+	g_free(uri);
+	g_free(collate_key);
+	return entry;
+}
+
 static gboolean
 model_reload_to(FivIoModel *self, GFile *directory,
-	GArray *subdirs, GArray *files, GError **error)
+	GPtrArray *subdirs, GPtrArray *files, GError **error)
 {
 	if (subdirs)
-		g_array_set_size(subdirs, 0);
+		g_ptr_array_set_size(subdirs, 0);
 	if (files)
-		g_array_set_size(files, 0);
+		g_ptr_array_set_size(files, 0);
 
 	GFileEnumerator *enumerator = g_file_enumerate_children(directory,
 		G_FILE_ATTRIBUTE_STANDARD_TYPE ","
 		G_FILE_ATTRIBUTE_STANDARD_NAME ","
+		G_FILE_ATTRIBUTE_STANDARD_SIZE ","
 		G_FILE_ATTRIBUTE_STANDARD_DISPLAY_NAME ","
 		G_FILE_ATTRIBUTE_STANDARD_TARGET_URI ","
 		G_FILE_ATTRIBUTE_STANDARD_IS_HIDDEN ","
@@ -3196,40 +3256,22 @@ model_reload_to(FivIoModel *self, GFile *directory,
 		if (self->filtering && g_file_info_get_is_hidden(info))
 			continue;
 
-		GArray *target = NULL;
+		GPtrArray *target = NULL;
 		if (g_file_info_get_file_type(info) == G_FILE_TYPE_DIRECTORY)
 			target = subdirs;
 		else if (!self->filtering ||
 			model_supports(self, g_file_info_get_name(info)))
 			target = files;
-		if (!target)
-			continue;
 
-		FivIoModelEntry entry = {.uri = g_file_get_uri(child),
-			.target_uri = g_strdup(g_file_info_get_attribute_string(
-				info, G_FILE_ATTRIBUTE_STANDARD_TARGET_URI)),
-			.display_name = g_strdup(g_file_info_get_display_name(info))};
-		GDateTime *mtime = g_file_info_get_modification_date_time(info);
-		if (mtime) {
-			entry.mtime_msec = g_date_time_to_unix(mtime) * 1000 +
-				g_date_time_get_microsecond(mtime) / 1000;
-			g_date_time_unref(mtime);
-		}
-
-		gchar *parse_name = g_file_get_parse_name(child);
-		// TODO(p): Make it possible to use g_utf8_collate_key() instead,
-		// which does not use natural sorting.
-		entry.collate_key = g_utf8_collate_key_for_filename(parse_name, -1);
-		g_free(parse_name);
-
-		g_array_append_val(target, entry);
+		if (target)
+			g_ptr_array_add(target, model_entry_new(child, info));
 	}
 	g_object_unref(enumerator);
 
 	if (subdirs)
-		g_array_sort_with_data(subdirs, model_compare, self);
+		g_ptr_array_sort_with_data(subdirs, model_compare, self);
 	if (files)
-		g_array_sort_with_data(files, model_compare, self);
+		g_ptr_array_sort_with_data(files, model_compare, self);
 	return TRUE;
 }
 
@@ -3248,8 +3290,8 @@ model_reload(FivIoModel *self, GError **error)
 static void
 model_resort(FivIoModel *self)
 {
-	g_array_sort_with_data(self->subdirs, model_compare, self);
-	g_array_sort_with_data(self->files, model_compare, self);
+	g_ptr_array_sort_with_data(self->subdirs, model_compare, self);
+	g_ptr_array_sort_with_data(self->files, model_compare, self);
 
 	g_signal_emit(self, model_signals[FILES_CHANGED], 0);
 	g_signal_emit(self, model_signals[SUBDIRECTORIES_CHANGED], 0);
@@ -3262,13 +3304,13 @@ static GFile *
 model_last_deep_subdirectory(FivIoModel *self, GFile *directory)
 {
 	GFile *result = NULL;
-	GArray *subdirs = model_entry_array_new();
+	GPtrArray *subdirs = model_entry_array_new();
 	if (!model_reload_to(self, directory, subdirs, NULL, NULL))
 		goto out;
 
 	if (subdirs->len) {
-		GFile *last = g_file_new_for_uri(
-			g_array_index(subdirs, FivIoModelEntry, subdirs->len - 1).uri);
+		FivIoModelEntry *entry = g_ptr_array_index(subdirs, subdirs->len - 1);
+		GFile *last = g_file_new_for_uri(entry->uri);
 		result = model_last_deep_subdirectory(self, last);
 		g_object_unref(last);
 	} else {
@@ -3276,7 +3318,7 @@ model_last_deep_subdirectory(FivIoModel *self, GFile *directory)
 	}
 
 out:
-	g_array_free(subdirs, TRUE);
+	g_ptr_array_free(subdirs, TRUE);
 	return result;
 }
 
@@ -3290,13 +3332,13 @@ fiv_io_model_get_previous_directory(FivIoModel *self)
 		return NULL;
 
 	GFile *result = NULL;
-	GArray *subdirs = model_entry_array_new();
+	GPtrArray *subdirs = model_entry_array_new();
 	if (!model_reload_to(self, parent_directory, subdirs, NULL, NULL))
 		goto out;
 
 	for (gsize i = 0; i < subdirs->len; i++) {
-		GFile *file = g_file_new_for_uri(
-			g_array_index(subdirs, FivIoModelEntry, i).uri);
+		FivIoModelEntry *entry = g_ptr_array_index(subdirs, i);
+		GFile *file = g_file_new_for_uri(entry->uri);
 		if (g_file_equal(file, self->directory)) {
 			g_object_unref(file);
 			break;
@@ -3315,7 +3357,7 @@ fiv_io_model_get_previous_directory(FivIoModel *self)
 
 out:
 	g_object_unref(parent_directory);
-	g_array_free(subdirs, TRUE);
+	g_ptr_array_free(subdirs, TRUE);
 	return result;
 }
 
@@ -3328,14 +3370,14 @@ model_next_directory_within_parents(FivIoModel *self, GFile *directory)
 		return NULL;
 
 	GFile *result = NULL;
-	GArray *subdirs = model_entry_array_new();
+	GPtrArray *subdirs = model_entry_array_new();
 	if (!model_reload_to(self, parent_directory, subdirs, NULL, NULL))
 		goto out;
 
 	gboolean found_self = FALSE;
 	for (gsize i = 0; i < subdirs->len; i++) {
-		result = g_file_new_for_uri(
-			g_array_index(subdirs, FivIoModelEntry, i).uri);
+		FivIoModelEntry *entry = g_ptr_array_index(subdirs, i);
+		result = g_file_new_for_uri(entry->uri);
 		if (found_self)
 			goto out;
 
@@ -3347,7 +3389,7 @@ model_next_directory_within_parents(FivIoModel *self, GFile *directory)
 
 out:
 	g_object_unref(parent_directory);
-	g_array_free(subdirs, TRUE);
+	g_ptr_array_free(subdirs, TRUE);
 	return result;
 }
 
@@ -3357,8 +3399,8 @@ fiv_io_model_get_next_directory(FivIoModel *self)
 	g_return_val_if_fail(FIV_IS_IO_MODEL(self), NULL);
 
 	if (self->subdirs->len) {
-		return g_file_new_for_uri(
-			g_array_index(self->subdirs, FivIoModelEntry, 0).uri);
+		FivIoModelEntry *entry = g_ptr_array_index(self->subdirs, 0);
+		return g_file_new_for_uri(entry->uri);
 	}
 
 	return model_next_directory_within_parents(self, self->directory);
@@ -3376,8 +3418,8 @@ fiv_io_model_finalize(GObject *gobject)
 
 	g_clear_object(&self->directory);
 	g_clear_object(&self->monitor);
-	g_array_free(self->subdirs, TRUE);
-	g_array_free(self->files, TRUE);
+	g_ptr_array_free(self->subdirs, TRUE);
+	g_ptr_array_free(self->files, TRUE);
 
 	G_OBJECT_CLASS(fiv_io_model_parent_class)->finalize(gobject);
 }
@@ -3510,18 +3552,18 @@ fiv_io_model_get_location(FivIoModel *self)
 	return self->directory;
 }
 
-const FivIoModelEntry *
+FivIoModelEntry *const *
 fiv_io_model_get_files(FivIoModel *self, gsize *len)
 {
 	*len = self->files->len;
-	return (const FivIoModelEntry *) self->files->data;
+	return (FivIoModelEntry *const *) self->files->pdata;
 }
 
-const FivIoModelEntry *
+FivIoModelEntry *const *
 fiv_io_model_get_subdirs(FivIoModel *self, gsize *len)
 {
 	*len = self->subdirs->len;
-	return (const FivIoModelEntry *) self->subdirs->data;
+	return (FivIoModelEntry *const *) self->subdirs->pdata;
 }
 
 // --- Export ------------------------------------------------------------------

fiv-io.h

@@ -135,20 +135,21 @@ GFile *fiv_io_model_get_previous_directory(FivIoModel *self);
 /// Returns the next VFS directory in order, or NULL.
 GFile *fiv_io_model_get_next_directory(FivIoModel *self);
 
-// TODO(p): Turn this into a reference-counted object.
-//  - If using g_rc_box_*(), we should wrap the {_acquire,_release_full}()
-//    functions as fiv_io_model_entry_{ref,unref}().
-//  - Ideally, all the strings would follow the struct immediately.
+// These objects are reference-counted using GRcBox.
 typedef struct {
-	gchar *uri;                         ///< GIO URI
-	gchar *target_uri;                  ///< GIO URI for any target
-	gchar *display_name;                ///< Label for the file
-	gchar *collate_key;                 ///< Collate key for the filename
+	const char *uri;                    ///< GIO URI
+	const char *target_uri;             ///< GIO URI for any target
+	const char *display_name;           ///< Label for the file
+	const char *collate_key;            ///< Collate key for the filename
+	guint64 filesize;                   ///< Filesize in bytes
 	gint64 mtime_msec;                  ///< Modification time in milliseconds
 } FivIoModelEntry;
 
-const FivIoModelEntry *fiv_io_model_get_files(FivIoModel *self, gsize *len);
-const FivIoModelEntry *fiv_io_model_get_subdirs(FivIoModel *self, gsize *len);
+#define fiv_io_model_entry_ref(e)    g_rc_box_acquire(e)
+#define fiv_io_model_entry_unref(e)  g_rc_box_release(e)
+
+FivIoModelEntry *const *fiv_io_model_get_files(FivIoModel *self, gsize *len);
+FivIoModelEntry *const *fiv_io_model_get_subdirs(FivIoModel *self, gsize *len);
 
 // --- Export ------------------------------------------------------------------
 

fiv-sidebar.c

@@ -357,10 +357,10 @@ update_location(FivSidebar *self)
 		gtk_container_add(GTK_CONTAINER(self->listbox), row);
 
 	gsize len = 0;
-	const FivIoModelEntry *subdirs =
+	FivIoModelEntry *const *subdirs =
 		fiv_io_model_get_subdirs(self->model, &len);
 	for (gsize i = 0; i < len; i++) {
-		GFile *file = g_file_new_for_uri(subdirs[i].uri);
+		GFile *file = g_file_new_for_uri(subdirs[i]->uri);
 		if ((row = create_row(self, file, "go-down-symbolic")))
 			gtk_container_add(GTK_CONTAINER(self->listbox), row);
 		g_object_unref(file);

fiv-thumbnail.c

@@ -524,7 +524,7 @@ fiv_thumbnail_produce(GFile *target, FivThumbnailSize max_size, GError **error)
 	g_string_append_printf(
 		thum, "%s%c%ld%c", THUMB_MTIME, 0, (long) st.st_mtime, 0);
 	g_string_append_printf(
-		thum, "%s%c%ld%c", THUMB_SIZE, 0, (long) filesize, 0);
+		thum, "%s%c%llu%c", THUMB_SIZE, 0, (unsigned long long) filesize, 0);
 
 	if (cairo_surface_get_type(surface) == CAIRO_SURFACE_TYPE_IMAGE) {
 		g_string_append_printf(thum, "%s%c%d%c", THUMB_IMAGE_WIDTH, 0,
@@ -563,9 +563,16 @@ fiv_thumbnail_produce(GFile *target, FivThumbnailSize max_size, GError **error)
 	return max_size_surface;
 }
 
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+
+typedef struct {
+	const char *uri;                    ///< Target URI
+	time_t mtime;                       ///< File modification time
+	guint64 size;                       ///< File size
+} Stat;
+
 static bool
-check_wide_thumbnail_texts(GBytes *thum, const char *target, time_t mtime,
-	bool *sRGB)
+check_wide_thumbnail_texts(GBytes *thum, const Stat *st, bool *sRGB)
 {
 	gsize len = 0;
 	const gchar *s = g_bytes_get_data(thum, &len), *end = s + len;
@@ -579,11 +586,14 @@ check_wide_thumbnail_texts(GBytes *thum, const char *target, time_t mtime,
 			continue;
 		} else if (!strcmp(key, THUMB_URI)) {
 			have_uri = true;
-			if (strcmp(target, s))
+			if (strcmp(st->uri, s))
 				return false;
 		} else if (!strcmp(key, THUMB_MTIME)) {
 			have_mtime = true;
-			if (atol(s) != mtime)
+			if (atol(s) != st->mtime)
+				return false;
+		} else if (!strcmp(key, THUMB_SIZE)) {
+			if (strtoull(s, NULL, 10) != st->size)
 				return false;
 		} else if (!strcmp(key, THUMB_COLORSPACE))
 			*sRGB = !strcmp(s, THUMB_COLORSPACE_SRGB);
@@ -594,8 +604,7 @@ check_wide_thumbnail_texts(GBytes *thum, const char *target, time_t mtime,
 }
 
 static cairo_surface_t *
-read_wide_thumbnail(
-	const char *path, const char *uri, time_t mtime, GError **error)
+read_wide_thumbnail(const char *path, const Stat *st, GError **error)
 {
 	gchar *thumbnail_uri = g_filename_to_uri(path, NULL, error);
 	if (!thumbnail_uri)
@@ -612,7 +621,7 @@ read_wide_thumbnail(
 	if (!thum) {
 		g_clear_error(error);
 		set_error(error, "not a thumbnail");
-	} else if (!check_wide_thumbnail_texts(thum, uri, mtime, &sRGB)) {
+	} else if (!check_wide_thumbnail_texts(thum, st, &sRGB)) {
 		g_clear_error(error);
 		set_error(error, "mismatch");
 	} else {
@@ -629,11 +638,8 @@ read_wide_thumbnail(
 	return NULL;
 }
 
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-
 static cairo_surface_t *
-read_png_thumbnail(
-	const char *path, const char *uri, time_t mtime, GError **error)
+read_png_thumbnail(const char *path, const Stat *st, GError **error)
 {
 	cairo_surface_t *surface = fiv_io_open_png_thumbnail(path, error);
 	if (!surface)
@@ -650,18 +656,27 @@ read_png_thumbnail(
 	// but those aren't interesting currently (would be for fast previews).
 	const char *text_uri = g_hash_table_lookup(texts, THUMB_URI);
 	const char *text_mtime = g_hash_table_lookup(texts, THUMB_MTIME);
-	if (!text_uri || strcmp(text_uri, uri) ||
-		!text_mtime || atol(text_mtime) != mtime) {
+	const char *text_size = g_hash_table_lookup(texts, THUMB_SIZE);
+	if (!text_uri || strcmp(text_uri, st->uri) ||
+		!text_mtime || atol(text_mtime) != st->mtime) {
 		set_error(error, "mismatch or not a thumbnail");
 		cairo_surface_destroy(surface);
 		return NULL;
 	}
+	if (text_size && strtoull(text_size, NULL, 10) != st->size) {
+		set_error(error, "file size mismatch");
+		cairo_surface_destroy(surface);
+		return NULL;
+	}
 
 	return surface;
 }
 
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+
 cairo_surface_t *
-fiv_thumbnail_lookup(const char *uri, gint64 mtime_msec, FivThumbnailSize size)
+fiv_thumbnail_lookup(const char *uri, gint64 mtime_msec, guint64 filesize,
+	FivThumbnailSize size)
 {
 	g_return_val_if_fail(size >= FIV_THUMBNAIL_SIZE_MIN &&
 		size <= FIV_THUMBNAIL_SIZE_MAX, NULL);
@@ -673,6 +688,7 @@ fiv_thumbnail_lookup(const char *uri, gint64 mtime_msec, FivThumbnailSize size)
 
 	gchar *sum = g_compute_checksum_for_string(G_CHECKSUM_MD5, uri, -1);
 	gchar *thumbnails_dir = fiv_thumbnail_get_root();
+	const Stat st = {.uri = uri, .mtime = mtime_msec / 1000, .size = filesize};
 
 	// The lookup sequence is: nominal..max, then mirroring back to ..min.
 	cairo_surface_t *result = NULL;
@@ -685,7 +701,7 @@ fiv_thumbnail_lookup(const char *uri, gint64 mtime_msec, FivThumbnailSize size)
 		const char *name = fiv_thumbnail_sizes[use].thumbnail_spec_name;
 		gchar *wide = g_strconcat(thumbnails_dir, G_DIR_SEPARATOR_S "wide-",
 			name, G_DIR_SEPARATOR_S, sum, ".webp", NULL);
-		result = read_wide_thumbnail(wide, uri, mtime_msec / 1000, &error);
+		result = read_wide_thumbnail(wide, &st, &error);
 		if (error) {
 			g_debug("%s: %s", wide, error->message);
 			g_clear_error(&error);
@@ -701,7 +717,7 @@ fiv_thumbnail_lookup(const char *uri, gint64 mtime_msec, FivThumbnailSize size)
 
 		gchar *path = g_strconcat(thumbnails_dir, G_DIR_SEPARATOR_S,
 			name, G_DIR_SEPARATOR_S, sum, ".png", NULL);
-		result = read_png_thumbnail(path, uri, mtime_msec / 1000, &error);
+		result = read_png_thumbnail(path, &st, &error);
 		if (error) {
 			g_debug("%s: %s", path, error->message);
 			g_clear_error(&error);
@@ -734,7 +750,7 @@ print_error(GFile *file, GError *error)
 }
 
 static gchar *
-identify_wide_thumbnail(GMappedFile *mf, time_t *mtime, GError **error)
+identify_wide_thumbnail(GMappedFile *mf, Stat *st, GError **error)
 {
 	WebPDemuxer *demux = WebPDemux(&(WebPData) {
 		.bytes = (const uint8_t *) g_mapped_file_get_contents(mf),
@@ -760,7 +776,9 @@ identify_wide_thumbnail(GMappedFile *mf, time_t *mtime, GError **error)
 			if (!strcmp(key, THUMB_URI) && !uri)
 				uri = g_strdup(p);
 			if (!strcmp(key, THUMB_MTIME))
-				*mtime = atol(p);
+				st->mtime = atol(p);
+			if (!strcmp(key, THUMB_SIZE))
+				st->size = strtoull(p, NULL, 10);
 			key = NULL;
 		} else {
 			key = p;
@@ -778,16 +796,17 @@ static void
 check_wide_thumbnail(GFile *thumbnail, GError **error)
 {
 	// Not all errors are enough of a reason for us to delete something.
-	GError *tolerable = NULL;
+	GError *tolerable_error = NULL;
 	const char *path = g_file_peek_path(thumbnail);
-	GMappedFile *mf = g_mapped_file_new(path, FALSE, &tolerable);
+	GMappedFile *mf = g_mapped_file_new(path, FALSE, &tolerable_error);
 	if (!mf) {
-		print_error(thumbnail, tolerable);
+		print_error(thumbnail, tolerable_error);
 		return;
 	}
 
-	time_t target_mtime = 0;
-	gchar *target_uri = identify_wide_thumbnail(mf, &target_mtime, error);
+	// Note that we could enforce the presence of the size field in our spec.
+	Stat target_st = {.uri = NULL, .mtime = 0, .size = G_MAXUINT64};
+	gchar *target_uri = identify_wide_thumbnail(mf, &target_st, error);
 	g_mapped_file_unref(mf);
 	if (!target_uri)
 		return;
@@ -809,26 +828,32 @@ check_wide_thumbnail(GFile *thumbnail, GError **error)
 	GFile *target = g_file_new_for_uri(target_uri);
 	g_free(target_uri);
 	GFileInfo *info = g_file_query_info(target,
-		G_FILE_ATTRIBUTE_STANDARD_NAME "," G_FILE_ATTRIBUTE_TIME_MODIFIED,
-		G_FILE_QUERY_INFO_NONE, NULL, &tolerable);
+		G_FILE_ATTRIBUTE_STANDARD_NAME ","
+		G_FILE_ATTRIBUTE_STANDARD_SIZE ","
+		G_FILE_ATTRIBUTE_TIME_MODIFIED,
+		G_FILE_QUERY_INFO_NONE, NULL, &tolerable_error);
 	g_object_unref(target);
-	if (g_error_matches(tolerable, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) {
-		g_propagate_error(error, tolerable);
+	if (g_error_matches(tolerable_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) {
+		g_propagate_error(error, tolerable_error);
 		return;
-	} else if (tolerable) {
-		print_error(thumbnail, tolerable);
+	} else if (tolerable_error) {
+		print_error(thumbnail, tolerable_error);
 		return;
 	}
 
+	guint64 filesize = g_file_info_get_size(info);
 	GDateTime *mdatetime = g_file_info_get_modification_date_time(info);
 	g_object_unref(info);
 	if (!mdatetime) {
-		set_error(&tolerable, "cannot retrieve file modification time");
-		print_error(thumbnail, tolerable);
+		set_error(&tolerable_error, "cannot retrieve file modification time");
+		print_error(thumbnail, tolerable_error);
 		return;
 	}
-	if (g_date_time_to_unix(mdatetime) != target_mtime)
-		set_error(error, "mtime mismatch");
+	if (g_date_time_to_unix(mdatetime) != target_st.mtime)
+		set_error(error, "modification time mismatch");
+	else if (target_st.size != G_MAXUINT64 && filesize != target_st.size)
+		set_error(error, "file size mismatch");
+
 	g_date_time_unref(mdatetime);
 }
 

fiv-thumbnail.h

@@ -68,8 +68,8 @@ cairo_surface_t *fiv_thumbnail_produce_for_search(
 
 /// Retrieves a thumbnail of the most appropriate quality and resolution
 /// for the target file.
-cairo_surface_t *fiv_thumbnail_lookup(
-	const char *uri, gint64 mtime_msec, FivThumbnailSize size);
+cairo_surface_t *fiv_thumbnail_lookup(const char *uri,
+	gint64 mtime_msec, guint64 filesize, FivThumbnailSize size);
 
 /// Invalidate the wide thumbnail cache. May write to standard streams.
 void fiv_thumbnail_invalidate(void);

fiv.c

@@ -783,11 +783,11 @@ on_model_files_changed(FivIoModel *model, G_GNUC_UNUSED gpointer user_data)
 	g_return_if_fail(model == g.model);
 
 	gsize len = 0;
-	const FivIoModelEntry *files = fiv_io_model_get_files(g.model, &len);
+	FivIoModelEntry *const *files = fiv_io_model_get_files(g.model, &len);
 	g_ptr_array_free(g.files, TRUE);
 	g.files = g_ptr_array_new_full(len, g_free);
 	for (gsize i = 0; i < len; i++)
-		g_ptr_array_add(g.files, g_strdup(files[i].uri));
+		g_ptr_array_add(g.files, g_strdup(files[i]->uri));
 
 	update_files_index();