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

scripts/symbolize.py: accept -d <ELF_file> in addition to -d <dir> #1766

Merged
merged 1 commit into from
Sep 5, 2017

Conversation

jforissier
Copy link
Contributor

The -d option of symbolize.py normally expects one or more directories,
where the script will look for ELF files (TEE or TA, depending on the
input dump). For convenience, let's also accept paths to the actual ELF
files. Previously, the script would just ignore file arguments and
silently fail to resolve stack traces.

Reported-by: Lijianhui airbak.li@hisilicon.com
Signed-off-by: Jerome Forissier jerome.forissier@linaro.org

@jforissier
Copy link
Contributor Author

Ping?

@@ -94,6 +95,8 @@ def get_elf(self, elf_or_uuid):
if not elf_or_uuid.endswith('.elf'):
elf_or_uuid += '.elf'
for d in self._dirs:
if d.endswith(elf_or_uuid) and glob.glob(d):
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe I misunderstood what you was trying to do with glob.glob(d), but in case if it is a verification of file existence, maybe it's better to use d.is_file() (more obvious)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct. That would be os.path.isfile(d), but then we would need import os, I'm not sure it is worth it. Would you prefer that? I'm fine with both ways.

Copy link
Contributor

@igoropaniuk igoropaniuk Sep 4, 2017

Choose a reason for hiding this comment

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

@jforissier sorry for suggesting d.is_file(), found that it's possible to use it in python 3 only (where OOP-friendly pathlib exists). I'm fine with both ways also, but IMHO usage of os.path.isfile(d) will help easily understand what this code supposed to do (at least for me :) )

Anyway:
Reviewed-by: Igor Opaniuk <igor.opaniuk@linaro.org>

The -d option of symbolize.py normally expects one or more directories,
where the script will look for ELF files (TEE or TA, depending on the
input dump). For convenience, let's also accept paths to the actual ELF
files. Previously, the script would just ignore file arguments and
silently fail to resolve stack traces.

Reported-by: Lijianhui <airbak.li@hisilicon.com>
Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Reviewed-by: Igor Opaniuk <igor.opaniuk@linaro.org>
@jforissier jforissier merged commit 157e621 into OP-TEE:master Sep 5, 2017
@jforissier jforissier deleted the symbolize-d-file branch September 5, 2017 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants