Claude Code Review Report

Recently, I ran Claude Code across several projects(like image-rs, lofty of symphonia) to look for potential issues.

In these projects, a significant portion of reported findings turned out to be incorrect or minor false positives, but a non-trivial subset was valid and included real issues ranging from incorrect comments to actual logic bugs. As a result, the findings generally require manual validation to separate noise from actionable problems.

Full Reports (findings-interactive.html - interactive report view for manual inspection, findings-short.md - compact list ready to copy into GitHub, findings-table.html - compact tabular report version):

findings-interactive.html
findings-short.md
findings-table.html

Example findings (this is only a subset of the findings, specifically those most likely to be actual bugs; for the full list, see the reports above):

=================================

DEAD_1 NEGLIGIBLE

Description: Loop at lines 4211-4218 iterates while f != NULL and only exits when g_file_get_parent returns NULL (assignment at line 4216). After the loop, the block if (f) { g_object_unref (f); } is unreachable because f is provably NULL at that point. Cosmetic.

Locations:

libnemo-private/nemo-file-operations.c:4205-4225
 4205 | static gboolean
 4206 | test_dir_is_parent (GFile *child, GFile *root)
 4207 | {
 4208 | 	GFile *f, *tmp;
 4209 | 
 4210 | 	f = g_file_dup (child);
 4211 | 	while (f) {
 4212 | 		if (g_file_equal (f, root)) {
 4213 | 			g_object_unref (f);
 4214 | 			return TRUE;
 4215 | 		}
 4216 | 		tmp = f;
 4217 | 		f = g_file_get_parent (f);
 4218 | 		g_object_unref (tmp);
 4219 | 	}
 4220 | 	if (f) {
 4221 | 		g_object_unref (f);
 4222 | 	}
 4223 | 	return FALSE;
 4224 | }
 4225 | 

Fix: Remove the unreachable if (f) { g_object_unref (f); } block following the while loop.

LOGIC_1 HIGH

Description: nemo_directory_notify_files_removed_by_uri builds a file list and then calls nemo_directory_notify_files_changed instead of nemo_directory_notify_files_removed. The two sibling helpers immediately above (nemo_directory_notify_files_added_by_uri on line 1045 calls _added, nemo_directory_notify_files_changed_by_uri on line 1109 calls _changed) follow the expected naming. As written, files removed via the URI entry point are merely marked changed — they are never marked gone, never removed from directory lists, and the UI keeps stale entries. This is a copy/paste mistake.

Locations:

libnemo-private/nemo-directory.c:1166-1174
 1166 | void
 1167 | nemo_directory_notify_files_removed_by_uri (GList *uris)
 1168 | {
 1169 | 	GList *files;
 1170 | 
 1171 | 	files = nemo_file_list_from_uris (uris);
 1172 | 	nemo_directory_notify_files_changed (files);
 1173 | 	g_list_free_full (files, g_object_unref);
 1174 | }

Fix: Replace the call on line 1172:

nemo_directory_notify_files_removed (files);

LOGIC_2 HIGH

Description: The conditional response != GTK_RESPONSE_CANCEL || response != GTK_RESPONSE_NONE is tautologically TRUE — no single value can simultaneously equal both constants. The intended check is &&. As a result, the apply_to_all field is queried and stored even when the user clicked Cancel or closed the dialog (GTK_RESPONSE_NONE / DELETE_EVENT). For a batch copy/move that hits a conflict, clicking Cancel can still leave apply_to_all set, which then suppresses further conflict dialogs for the rest of the job.

Locations:

libnemo-private/nemo-file-operations.c:4327-4335
 4327 | 	if (response == CONFLICT_RESPONSE_RENAME) {
 4328 | 		data->resp_data->new_name =
 4329 | 			nemo_file_conflict_dialog_get_new_name (NEMO_FILE_CONFLICT_DIALOG (dialog));
 4330 | 	} else if (response != GTK_RESPONSE_CANCEL ||
 4331 | 		   response != GTK_RESPONSE_NONE) {
 4332 | 		   data->resp_data->apply_to_all =
 4333 | 			   nemo_file_conflict_dialog_get_apply_to_all
 4334 | 				(NEMO_FILE_CONFLICT_DIALOG (dialog));
 4335 | 	}

Fix: Change the || on line 4330 to &&:

} else if (response != GTK_RESPONSE_CANCEL &&
           response != GTK_RESPONSE_NONE) {
    data->resp_data->apply_to_all =
        nemo_file_conflict_dialog_get_apply_to_all (NEMO_FILE_CONFLICT_DIALOG (dialog));
}

LOGIC_5 LOW

Description: Statement ends with , instead of ;. The line silently becomes part of a comma expression with the subsequent statement (pixbuf = nemo_file_get_icon_pixbuf (...)). It compiles, evaluates left-to-right, and currently produces the same observable behavior — but the construct is unambiguously a typo, defeats compiler diagnostics that would otherwise catch a misplaced , in similar contexts, and will surprise anyone editing nearby code.

Locations:

libnemo-private/nemo-file-conflict-dialog.c:224
  224 |     ui_scale = gtk_widget_get_scale_factor (fcd->details->titles_vbox),

Fix: Change the trailing , on line 224 to ;.

MEM_1 HIGH

Description: Inside nemo_window_slot_dispose, slot->location is g_object_ref'd instead of g_object_unref'd. The TODO comment above the line ("why do we ref here, instead of unreffing?") acknowledges the suspicion but the bug has been left in. Every navigated location is leaked one extra reference per slot disposal, and the GFile (plus everything it holds) is never released.

Locations:

src/nemo-window-slot.c:417-421
  417 | 	if (slot->location) {
  418 | 		/* TODO? why do we ref here, instead of unreffing?
  419 | 		 * It was already here before the slot migration, though */
  420 | 		g_object_ref (slot->location);
  421 | 	}

Fix: Replace the bogus ref with the proper cleanup:

if (slot->location) {
    g_object_unref (slot->location);
    slot->location = NULL;
}

OTHER_1 NEGLIGIBLE

Description: Variable named launchpad_sucks was apparently a developer placeholder / vent that shipped into stable code. Not a functional defect, but it appears in any backtrace that touches this function and is awkward in a release codebase.

Locations:

src/nemo-progress-ui-handler.c:117
  117 |     gchar *launchpad_sucks = THOU_TO_STR (self->priv->active_infos);

Fix: Rename to something descriptive like active_count_str or count_label.

PANIC_1 HIGH

Description: g_file_get_uri (selection) is called unconditionally on the error path, but the function is reachable when selection == NULL (the prior block at lines 405-409 explicitly handles that case by falling back to location). Whenever a user invokes the helper with only a directory location, a failure to launch the file browser will crash on the NULL dereference.

Locations:

src/nemo-desktop-application.c:405-419
  405 |     if (selection != NULL) {
  406 |         sel_list = g_list_prepend (sel_list, selection);
  407 |     } else if (location != NULL) {
  408 |         sel_list = g_list_prepend (sel_list, location);
  409 |     }
  410 | 
  411 |     if (!g_app_info_launch (appinfo, sel_list, NULL, &error)) {
  412 |         gchar *uri;
  413 | 
  414 |         uri = g_file_get_uri (selection);
  415 |         g_warning ("Could not launch file browser to display file: %s\n", uri);
  416 | 
  417 |         g_free (uri);
  418 |         g_clear_error (&error);
  419 |     }

Fix: Use whichever pointer is non-NULL when building the warning URI:

GFile *uri_src = (selection != NULL) ? selection : location;
uri = g_file_get_uri (uri_src);

Review the reported locations in libnemo-private/nemo-directory.c, libnemo-private/nemo-file-operations.c, libnemo-private/nemo-file-conflict-dialog.c, src/nemo-window-slot.c, src/nemo-progress-ui-handler.c, and src/nemo-desktop-application.c. Start by manually validating each finding against its surrounding function and existing ownership or response-handling patterns. Done means confirmed issues are corrected without introducing regressions and the relevant project checks pass.