Skip to content

unikernels/linux: remove always-true conditional in firecracker block#764

Closed
BRGOVIND wants to merge 1 commit into
urunc-dev:mainfrom
BRGOVIND:fix/remove-redundant-firecracker-check
Closed

unikernels/linux: remove always-true conditional in firecracker block#764
BRGOVIND wants to merge 1 commit into
urunc-dev:mainfrom
BRGOVIND:fix/remove-redundant-firecracker-check

Conversation

@BRGOVIND

Copy link
Copy Markdown

Fixes the redundant conditional inside MonitorBlockCli() in
pkg/unikontainers/unikernels/linux.go.

What

The case "firecracker": arm of the switch already guarantees that
l.Monitor == "firecracker" is true. The inner if l.Monitor == "firecracker"
check can therefore never evaluate to false, making it dead code.

Before:

case "firecracker":
    for _, aBlock := range l.Blk {
        id := aBlock.ID
        if l.Monitor == "firecracker" {   // always true
            id = "FC" + aBlock.ID
        }
        blkArgs = append(blkArgs, types.MonitorBlockArgs{ID: id, Path: aBlock.Source})
    }

After:

case "firecracker":
    for _, aBlock := range l.Blk {
        blkArgs = append(blkArgs, types.MonitorBlockArgs{
            ID:   "FC" + aBlock.ID,
            Path: aBlock.Source,
        })
    }

Testing

Existing unit tests cover this path. No behaviour change — the "FC" prefix was
always applied; this just removes the dead branch.

Closes #747

Inside MonitorBlockCli(), the firecracker switch case already
guarantees l.Monitor == firecracker, so the inner if-check is
redundant. Remove it and assign the FC prefix directly.

Signed-off-by: B R GOVIND <brgovind2005@gmail.com>
@netlify

netlify Bot commented Jun 14, 2026

Copy link
Copy Markdown

Deploy Preview for urunc canceled.

Name Link
🔨 Latest commit 10e4ff5
🔍 Latest deploy log https://app.netlify.com/projects/urunc/deploys/6a2f34b3408114000803c30d

@cmainas

cmainas commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Hello @BRGOVIND ,

please read the contribution guide.

@BRGOVIND

Copy link
Copy Markdown
Author

Hello @BRGOVIND ,

please read the contribution guide.

Hi @cmainas, thanks for the pointer. I've read the contribution guide and will make sure to follow it properly going forward. Sorry about that.

@cmainas cmainas added the duplicate This issue or pull request already exists label Jun 22, 2026
@cmainas

cmainas commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Duplicate with #748

@BRGOVIND BRGOVIND closed this Jun 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge duplicate This issue or pull request already exists

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor(unikernels): redundant if l.Monitor == "firecracker" inside firecracker case in Linux.MonitorBlockCli

2 participants