-
Notifications
You must be signed in to change notification settings - Fork 6.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Devicetree: edtlib: fix possible AttributeError in Property.description #51139
Devicetree: edtlib: fix possible AttributeError in Property.description #51139
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your fix!
We do not accept PRs with merge commit, though -- please fix your branch so it doesn't have the merge commit, and has a linear history of one commit on top of something in upstream instead.
https://www.youtube.com/watch?v=mnVNLM7ynMQ
Patch itself looks good, thanks
BTW, dtsh looks amazing! Please consider me officially interested in getting support for this into upstream Zephyr when you feel it has matured enough :). cc @gmarull |
dcdca66
to
f47a3cd
Compare
@mbolivar-nordic Thanks for the review, and the link: I also prefer a linear git history, but wasn't sure of your policy regarding force push, now I know. I should have red the contribution guidelines more closely, though. Sorry for this. [Edit: |
9384b43
to
4a611e0
Compare
Attempting to access the property Property.description when Property.spec.description is None would raise AttributeError: 'NoneType' object has no attribute 'strip'. Known properties that may not have a description (Property.spec.description is None): - 'compatible' for nodes such as / /soc /soc/timer@e000e010 /leds /pwmleds - 'reg' for nodes such as /soc/timer@e000e010 - 'status' for nodes such as /soc/timer@e000e010 - 'gpios' for nodes such as /leds/led_0 /buttons/button_0 - 'pwms' for nodes such as /pwmleds/pwm_led_0 This patch checks the PropertySpec.description attribute before calling strip(): will return None, and not raise AttributeError. Signed-off-by: Chris Duf <chris@openmarl.org> Co-authored-by: Gerard Marull-Paretas <gerard@teslabs.com>
4a611e0
to
42d321d
Compare
You may find this easier than just heading straight to the source code: https://docs.zephyrproject.org/latest/develop/west/extensions.html#adding-a-west-extension |
Yes, sure, thanks. OTOH, considering my weak Python-Fu, reading some good source code won't hurt ;-) |
@mbolivar-nordic, thanks for the kind words.
At this point (or even before), I'd like to have as many bug reports as possible, for as many different configurations as possible: is it appropriate to post an humble announce to Also at this point, I may open a draft PR to start working on the Thanks. -- |
Yep, of course! I would recommend sending email to
Yep, a draft PR is fine. Here is a (short!) talk which gives an overview of the upstreaming process: https://www.youtube.com/watch?v=mnVNLM7ynMQ |
@mbolivar-nordic , sorry for the delay, I've been overwhelmed at work, then I needed some rest a little farther from keyboards. And apart from a couple updates, mostly to cope with a few edtlib API changes, dtsh did not really mature, until I eventually start to work on the I've updated design and implementation with some lessons learnt from the prototype, and tried to come to a sizeable PR that still implements enough to prove tasteful or useful: #59863. BTW:
Kind regards, |
Attempting to access the Python property
Property.description
whenProperty.spec.description
isNone
would raise:Known DT properties that may not have a description (
Property.spec.description
isNone
):compatible
for nodes such as/
,/soc
,/soc/timer@e000e010
,/leds
or/pwmleds
reg
for nodes such as/soc/timer@e000e010
status
for nodes such as/soc/timer@e000e010
gpios
for nodes such as/leds/led_0
or/buttons/button_0
pwms
for nodes such as/pwmleds/pwm_led_0
This patch checks the
PropertySpec.description
attribute before callingstrip()
: will returnNone
, and not raiseAttributeError
.NOTE: I'm not aware of any legit
edtlib
use (e.g. by scripts in zephyr/scripts) that may trigger this issue, in particular I've never faced thisAttributeError
when building a Zephyr application. My use-case is slightly different (a shell-like interface to a devicetree and its bindings, dtsh)Thanks.
--
chris