Page MenuHomeFreeBSD

cam: Initialize wired to false
ClosedPublic

Authored by imp on Nov 29 2021, 2:03 PM.

Details

Reviewers
mjg
Group Reviewers
cam
Commits
rG3846662dab2c: cam: Initialize wired to false
Summary

As part of converting the code to a while loop, the unconditional
initialization of wired to false was lost.

Sponsored by: Netflix

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

imp requested review of this revision.Nov 29 2021, 2:03 PM
jrtc27 added inline comments.
sys/cam/cam_periph.c
615

The control flow here is also a bit weird, would make more sense to me as:

while (resource_find_dev(...) == 0 && !wired) {
    if (...) {
        wired = true;
    }
    ...
    if (...) {
        wired = true;
    }
}
if (wired)
    unit = dunit;
sys/cam/cam_periph.c
615

Or probably !wired && resource_find_dev(...) == 0 so it doesn't mess with dunit after wired becomes true

sys/cam/cam_periph.c
615

Yea, maybe true. Also true that it likely is more likely to introduce a new bug. I'll look at making that structural change in a separate commit that i can take more time to validate so there aren't other hidden dragons... like the fact that we hit continue and reset wired to false which would
break your suggested reordering. Otherwise we would just do 'break' at that point.

sys/cam/cam_periph.c
615

I see, initially read it as iterating over property names and thus only one would ever be true... but indeed that's not what it does. Agreed it's not for this diff.

As noted in the mail I don't know what this code is supposed to be doing, so I'm not going to review anything.

However, setting 'wired = false' upfront restores the previously seen behavior and fixes the immediate regression, so I would say this should be committed and any other tinkering should come in later.

This revision was not accepted when it landed; it landed in state Needs Review.Nov 30 2021, 9:43 PM
This revision was automatically updated to reflect the committed changes.