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

Try get terminal size of "/dev/tty" if stdout fails #627

Merged
merged 2 commits into from
Jan 3, 2024

Conversation

imwints
Copy link
Contributor

@imwints imwints commented Sep 22, 2023

LLDB does not set the terminal sizes for stdout so try to get them for "/dev/tty". This helps to get back traces on arm64 where gdb is not supported

@imwints imwints changed the title Try get terminal size of "/dev/tty" of stdout fails Try get terminal size of "/dev/tty" if stdout fails Sep 22, 2023
Copy link
Owner

@aristocratos aristocratos left a comment

Choose a reason for hiding this comment

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

Great idea!

But I think this could be reworked to be a bit more efficient.
For example if the deafult STDOUT_FILENO fails, setting a bool that switches over to using /dev/tty instead.

@imwints
Copy link
Contributor Author

imwints commented Oct 1, 2023

@aristocratos

Great idea!

But I think this could be reworked to be a bit more efficient. For example if the deafult STDOUT_FILENO fails, setting a bool that switches over to using /dev/tty instead.

I can't quite follow what you mean by that. Currently it is checked if STDOUT_FILENO can be used to get valid terminal dimensions, if it's not the case it tries to fall back to /dev/tty ...

Where would the bool come into play?

@aristocratos
Copy link
Owner

Where would the bool come into play?

Term::refresh is called at a minimum once every update cycle, since SIGWINCH isn't always reliable.
So if we are running on a system where STDOUT_FILENO fails to get current size, we will continue to make that same syscall every time we run Term::refresh even though it's gonna fail every time.

Syscalls cost a lot more than branching.

@imwints imwints force-pushed the lldb-term-size branch 2 times, most recently from fee3245 to 0293f0d Compare October 3, 2023 12:32
@imwints
Copy link
Contributor Author

imwints commented Oct 3, 2023

@aristocratos
Yeah this makes sense. Addressed this in the latest commit

LLDB does not set the terminal sizes for stdout so try to get them for
"/dev/tty". This helps to get back traces on arm64 where gdb is
not supported
@aristocratos aristocratos merged commit 32b6622 into aristocratos:main Jan 3, 2024
51 checks passed
@imwints imwints deleted the lldb-term-size branch January 7, 2024 19:54
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