Changeset View
Changeset View
Standalone View
Standalone View
sys/cam/cam_periph.c
Show First 20 Lines • Show All 594 Lines • ▼ Show 20 Lines | camperiphnextunit(struct periph_driver *p_drv, u_int newunit, bool wired, | ||||
} | } | ||||
return (newunit); | return (newunit); | ||||
} | } | ||||
static u_int | static u_int | ||||
camperiphunit(struct periph_driver *p_drv, path_id_t pathid, | camperiphunit(struct periph_driver *p_drv, path_id_t pathid, | ||||
target_id_t target, lun_id_t lun, const char *sn) | target_id_t target, lun_id_t lun, const char *sn) | ||||
{ | { | ||||
bool wired; | bool wired = false; | ||||
u_int unit; | u_int unit; | ||||
int i, val, dunit; | int i, val, dunit; | ||||
const char *dname, *strval; | const char *dname, *strval; | ||||
char pathbuf[32], *periph_name; | char pathbuf[32], *periph_name; | ||||
periph_name = p_drv->driver_name; | periph_name = p_drv->driver_name; | ||||
snprintf(pathbuf, sizeof(pathbuf), "scbus%d", pathid); | snprintf(pathbuf, sizeof(pathbuf), "scbus%d", pathid); | ||||
unit = 0; | unit = 0; | ||||
i = 0; | i = 0; | ||||
dname = periph_name; | dname = periph_name; | ||||
while (resource_find_dev(&i, dname, &dunit, NULL, NULL) == 0) { | while (resource_find_dev(&i, dname, &dunit, NULL, NULL) == 0) { | ||||
wired = false; | wired = false; | ||||
jrtc27: The control flow here is also a bit weird, would make more sense to me as:
```
while… | |||||
Not Done Inline ActionsOr probably !wired && resource_find_dev(...) == 0 so it doesn't mess with dunit after wired becomes true jrtc27: Or probably `!wired && resource_find_dev(...) == 0` so it doesn't mess with dunit after wired… | |||||
Done Inline ActionsYea, 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 imp: Yea, maybe true. Also true that it likely is more likely to introduce a new bug. I'll look at… | |||||
Not Done Inline ActionsI 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. jrtc27: I see, initially read it as iterating over property names and thus only one would ever be true.. | |||||
if (resource_string_value(dname, dunit, "at", &strval) == 0) { | if (resource_string_value(dname, dunit, "at", &strval) == 0) { | ||||
if (strcmp(strval, pathbuf) != 0) | if (strcmp(strval, pathbuf) != 0) | ||||
continue; | continue; | ||||
wired = true; | wired = true; | ||||
} | } | ||||
if (resource_int_value(dname, dunit, "target", &val) == 0) { | if (resource_int_value(dname, dunit, "target", &val) == 0) { | ||||
if (val != target) | if (val != target) | ||||
continue; | continue; | ||||
▲ Show 20 Lines • Show All 1,574 Lines • Show Last 20 Lines |
The control flow here is also a bit weird, would make more sense to me as: