Two more issues found by Claude#1189
Two more issues found by Claude#1189vojtechtrefny wants to merge 2 commits intostoraged-project:masterfrom
Conversation
The fdisk_unref_partition() call was commented out due to a double-free bug in libfdisk (fixed in util-linux 2.35). Add a version-guarded unref to prevent the leak on fixed versions while preserving the workaround for older ones. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The g_free(host_id_val) call is unreachable with a non-NULL value since it's inside a `if (host_id_val == NULL)` block. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughTwo cleanup-related bug fixes: removed unconditional memory cleanup call in NVMe fabrics error path; added version-conditional cleanup in partition management to prevent double-free issues on older libfdisk versions. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/plugins/part.c (1)
1715-1718: Version-guarded unref correctly fixes the leak on libfdisk >= 2.35.The fix properly restores
fdisk_unref_partition()behind a version guard, avoiding the reference leak on libfdisk ≥ 2.35 while preserving the workaround for the double-free bug in older versions. The code change is correct.However, several error paths in
bd_part_resize_part()(lines 1588, 1609, 1640, 1672, 1684, 1697, 1708) callfdisk_unref_partition(pa)unconditionally. Sincepaoriginates from the samefdisk_get_partition()call regardless of success or failure, these error paths should also apply the version guard for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugins/part.c` around lines 1715 - 1718, In bd_part_resize_part(), several error paths unconditionally call fdisk_unref_partition(pa) which contradicts the version-guarded unref used elsewhere; update each error path (the calls at the error exits near the checks at lines handling failures after fdisk_get_partition()) to only call fdisk_unref_partition(pa) when fdisk_version >= 2350 (or use the same fdisk_version check used in the existing fix), i.e., wrap those fdisk_unref_partition(pa) invocations in the same version guard or replace them with the guarded helper so pa is only unref'd for libfdisk >= 2.35 while preserving the older workaround for earlier versions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/plugins/part.c`:
- Around line 1715-1718: In bd_part_resize_part(), several error paths
unconditionally call fdisk_unref_partition(pa) which contradicts the
version-guarded unref used elsewhere; update each error path (the calls at the
error exits near the checks at lines handling failures after
fdisk_get_partition()) to only call fdisk_unref_partition(pa) when fdisk_version
>= 2350 (or use the same fdisk_version check used in the existing fix), i.e.,
wrap those fdisk_unref_partition(pa) invocations in the same version guard or
replace them with the guarded helper so pa is only unref'd for libfdisk >= 2.35
while preserving the older workaround for earlier versions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1dfd12d9-0721-46c0-bbcb-306468df758e
📒 Files selected for processing (2)
src/plugins/nvme/nvme-fabrics.csrc/plugins/part.c
💤 Files with no reviewable changes (1)
- src/plugins/nvme/nvme-fabrics.c
I think we are close to fixing everything :-)
Summary by CodeRabbit