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

Crash inside eshell with memory allocation error #707

Closed
sunlin7 opened this issue Apr 29, 2024 · 5 comments
Closed

Crash inside eshell with memory allocation error #707

sunlin7 opened this issue Apr 29, 2024 · 5 comments

Comments

@sunlin7
Copy link

sunlin7 commented Apr 29, 2024

Hi,
I tried difft inside eshell and got crash, just a simple diff two files:

$ difft fileA fileB
memory allocation of 9223372036854775803 bytes failed
aborted

The eshell is built-in emacs-30.0.50, and difft is the Difftastic 0.57.0 (00e837a 2024-04-01, built with rustc 1.65.0).

@Wilfred
Copy link
Owner

Wilfred commented Apr 30, 2024

Could you share the files that caused this issue?

@sunlin7
Copy link
Author

sunlin7 commented Apr 30, 2024

Hi @Wilfred
I attach a.el and b.el which will lead a crash message in my local with eshell inside emacs.
test.TGZ

$ emacs -nw -q # and start eshell
/tmp $ difft a.el b.el
b.el --- Emacs Lisp
File permissions changed from 100664 to 100600.
memory allocation of 9223372036854775804 bytes failed
aborted

@bernhard-herzog
Copy link

With a debug build I get a more maningful error and backtrace:

3: difft::display::side_by_side::SourceDimensions::new
          at ./src/display/side_by_side.rs:185:31

That's

        let lhs_total_width = (terminal_width - SPACER.len()) / 2;

When running in eshell, terminal_width is 0 and SPACER.len() is
positive, hence the overflow.

AFAICT, terminal_width is 0, because in detect_display_width
in ./src/options.rs:864 there's

    if let Ok((columns, _rows)) = crossterm::terminal::size() {

When running in eshell, crossterm::terminal::size() returns Ok, but
with colums and rows both 0.

So the cause is that crossterm cannot correctly determine the size of
the terminal in eshell. The best fix is probably to fix crossterm, but
as a workaround, difftastic could check for a 0 width here and fall back
on e.g. looking at the COLUMNS environment variable which is set
correctly in eshell or the default value of 80.

@Wilfred
Copy link
Owner

Wilfred commented May 10, 2024

Reported upstream: crossterm-rs/crossterm#891

@sunlin7
Copy link
Author

sunlin7 commented May 11, 2024

Thank you all for the great works!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants