Attention is currently required from: Arthur Heymans, Christian Walter, Felix Held, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Shuo Liu, Tim Chu.
Patch set 2:Code-Review -1
2 comments:
Patchset:
This could result in endless loops.
File src/device/device_util.c:
Patch Set #2, Line 254: !is_root_device(dev)
This is an important exit condition for the loop, you cannot remove it.
I know it's not obvious when coming from other projects. But coreboot's
devicetree was originally designed with as few as possible NULL pointers
(because NULL pointers can introduce bugs). Hence there are no upstream
facing NULL pointers by design and you can't use them as exit condition
when walking up. OTOH, this design makes it possible to use upstream
facing pointers without the need for defensive checks (assuming a valid
devicetree).
For instance, the following would be a valid and sane implementation
in coreboot:
```
const struct device *dev_get_domain(const struct device *dev)
{
assert(dev); /* must be called with `dev != NULL` by contract */
for (; !is_root_device(dev); dev = dev->upstream->dev) {
if (dev->path.type == DEVICE_PATH_DOMAIN)
return dev;
}
return NULL;
}
```
By design, there is no need for runtime NULL checks.
There was nothing wrong with PS1, though, so I suggest to simply
return to that.
To view, visit change 81957. To unsubscribe, or for help writing mail filters, visit settings.