Skip to content
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

Python: fix errors found by type checker #1205

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

axel-h
Copy link
Member

@axel-h axel-h commented Feb 26, 2024

Fix errors found by type checker while working on #797

@axel-h axel-h added cleanup Cleanup of code, comments, docs ... hw-build do all sel4test hardware builds on this PR labels Feb 26, 2024
@axel-h axel-h changed the title Python: fix errors found by type cheker Python: fix errors found by type checker Feb 26, 2024
@axel-h axel-h mentioned this pull request Feb 26, 2024
@axel-h axel-h force-pushed the patch-axel-11-1 branch 6 times, most recently from ebe72ab to d51f5b6 Compare February 27, 2024 21:05
Signed-off-by: Axel Heider <axelheider@gmx.de>
Copy link
Contributor

@Indanz Indanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commit comments can be improved with explanations for the emtpy ones and fixed typos.

tools/hardware/memory.py Show resolved Hide resolved
@@ -189,7 +189,7 @@ def create_c_header_file(config, kernel_irqs: List, kernel_macros: Dict,
kernel_regions: List, physBase: int, physical_memory,
outputStream):

jinja_env = jinja2.Environment(loader=jinja2.BaseLoader, trim_blocks=True,
jinja_env = jinja2.Environment(loader=jinja2.BaseLoader(), trim_blocks=True,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Surely BaseLoader is the default, why pass it at all?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I just fixed the warning here. Explicitly setting this parameter might come from history following recommendations from older versions. Will have a look into this.

Comment on lines 94 to 98
node = self.get_path(path)
if node is not None:
ret.append(node)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can this return None?

Also, empty line added unnecessarily.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The static type annotation checker reported this based on the signatures. In this PR I left out addressing the semantics also.

Comment on lines 20 to 21
if cpus_node is None:
return []

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems better if get_path returns an empty array than checking for None everywhere...

tools/hardware/fdt.py Outdated Show resolved Hide resolved
axel-h added 4 commits May 5, 2024 22:15
Removing the function because it is not use anywhere. Not cloning the
owner seems an inconsistent behavior anyway.

Signed-off-by: Axel Heider <axelheider@gmx.de>
Support comparisson operations for derived objects only, and fail for
anything else. Currently there is no use case that requires duck
typing and stricter type checking allows catching potential errors.

Signed-off-by: Axel Heider <axelheider@gmx.de>
Signed-off-by: Axel Heider <axelheider@gmx.de>
Signed-off-by: Axel Heider <axelheider@gmx.de>
axel-h added 5 commits May 6, 2024 02:21
Signed-off-by: Axel Heider <axelheider@gmx.de>
The DTB appears broken if an item in the chosen node can't be
resolved. Raise an error instead of silently ignoring this,
as it might be a typo that must be fixed. Otherwise, non-existing
items should be removed from the chosen node in a overlay file.

Signed-off-by: Axel Heider <axelheider@gmx.de>
The return values is not used by any known callers of this function.
There is no sane generic way to handle return values on recursive
invocations, thus any visitor must handle this internally. This is
doable in Python easily, so there is no need for an additional
explicit context parameter either.

Signed-off-by: Axel Heider <axelheider@gmx.de>
Element 0 may not exist if the string is empty.

Signed-off-by: Axel Heider <axelheider@gmx.de>
Lists are ordered and enumerate() provides the index.

Signed-off-by: Axel Heider <axelheider@gmx.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Cleanup of code, comments, docs ... hw-build do all sel4test hardware builds on this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants