kerndat: remove redundant ret check in root_only_init() - #3130
Closed
Goyamjain06 wants to merge 1 commit into
Closed
kerndat: remove redundant ret check in root_only_init()#3130Goyamjain06 wants to merge 1 commit into
Goyamjain06 wants to merge 1 commit into
Conversation
The first !ret check in root_only_init() is always true, since ret is freshly initialized to 0 immediately before this check and no prior code path can change it. This makes the check redundant and was flagged by Codacy (pattern 2237, 'condition is always true/false', see issue checkpoint-restore#2120). Subsequent !ret && kerndat_*() checks later in the same function are untouched, as they correctly guard against proceeding after an earlier failure. Assisted-by: Claude Code:claude-sonnet-5 Signed-off-by: Goyam Jain <goyam24224@iiitd.ac.in>
Member
|
I actually like the current form, because it looks like other checks in this function, so it is easier to read this code. |
Author
|
Totally fair — appreciate the quick review! Makes sense to prioritize consistency with the surrounding checks over the strict redundancy. I'll leave this as-is and look at the other patterns flagged under #2120 with that same lens before proposing anything else. Thanks for the feedback! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this solve?
Codacy static analysis (pattern 2237, "condition is always true/false") flags a redundant condition in
root_only_init()incriu/kerndat.c, tracked in #2120.What changed
The first check in
root_only_init()was:Since
retis freshly initialized to0immediately before this check, and no code path between the initialization and this check can change it,!retis always true here — making the check dead/redundant.Changed to:
All subsequent
!ret && kerndat_*()checks later in the same function are intentionally left untouched, since they correctly guard against proceeding after an earlier failure (short-circuit "stop on first error" pattern).How to test
make— compiles cleanly with-Werror, no warnings.criu check— behaves identically before/after this change (verified in a Docker-based Ubuntu 22.04 environment).root_only_init()is exercised indirectly any time CRIU runs kerndat initialization as root.Related issue
Relates to #2120 (Codacy pattern 2237). This is one of several redundant conditions flagged by that issue; a previous attempt to fix several of them (#2141) went stale and was closed unmerged due to messy/unrebased commit history. This PR addresses just the kerndat.c instance cleanly, as a first, minimal, reviewable step.