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

[with patch, review/change] preparser hangs if line starts with ... #1596

Closed
wjp mannequin opened this issue Dec 24, 2007 · 3 comments
Closed

[with patch, review/change] preparser hangs if line starts with ... #1596

wjp mannequin opened this issue Dec 24, 2007 · 3 comments

Comments

@wjp
Copy link
Mannequin

wjp mannequin commented Dec 24, 2007

As reported by 'Octoploid' on IRC: The preparser crashes if a line starts with '...'.

This is caused by a string index in preparse_ellipsis() becoming negative and wrapping to the end of the string.

Patch attached. This makes preparse('...') return 'Ellipsis'. Not sure if that's the desired behaviour. Maybe a syntax error would be better?

Component: user interface

Issue created by migration from https://trac.sagemath.org/ticket/1596

@wjp wjp mannequin assigned williamstein Dec 24, 2007
@wjp
Copy link
Mannequin Author

wjp mannequin commented Dec 24, 2007

Attachment: preparse_ellipsis.patch.gz

@wjp wjp mannequin added p: blocker / 1 and removed p: major / 3 labels Dec 24, 2007
@sagetrac-mabshoff sagetrac-mabshoff mannequin added this to the sage-2.9.2 milestone Dec 24, 2007
@robertwb
Copy link
Contributor

robertwb commented Jan 4, 2008

comment:4

Attachment: 1596-ellipsis-bug.diff.gz

This patch fixes the problem.

I agree that a syntax error would be preferable, see second patch (to replace the first). I also made the error on [1..] more explicit.

@robertwb robertwb changed the title preparser hangs if line starts with ... [with patch, review/change] preparser hangs if line starts with ... Jan 4, 2008
@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Jan 4, 2008

comment:5

Looks good to me. Merged in Sage 2.9.2.rc1.

@sagetrac-mabshoff sagetrac-mabshoff mannequin removed the s: needs review label Jan 4, 2008
@sagetrac-mabshoff sagetrac-mabshoff mannequin closed this as completed Jan 4, 2008
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

2 participants