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.

Sign up for GitHub

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

Jump to bottom

interp: panic on division by zero #892

Closed
dankegel opened this issue Jun 29, 2022 · 1 comment · Fixed by #934
Closed

interp: panic on division by zero #892

dankegel opened this issue Jun 29, 2022 · 1 comment · Fixed by #934
Labels
bug good first issue

Comments

@dankegel
Copy link

dankegel commented Jun 29, 2022 •
edited
Loading

Given the invalid command

let x/x

bash rightfully complains:

$ let x/x
bash: let: x/x: division by 0 (error token is "x")

but gosh panics:

$ go run cmd/gosh/main.go
$ let x/x
panic: runtime error: integer divide by zero

goroutine 1 [running]:
mvdan.cc/sh/v3/expand.binArit(0x111540?, 0x589de0?, 0xc000122f28?)
	/home/dank/src/sh/expand/arith.go:173 +0x245
mvdan.cc/sh/v3/expand.Arithm(0x0?, {0x589840?, 0xc000078480?})
	/home/dank/src/sh/expand/arith.go:95 +0x353
...

This happens in v3.0.0 and master. I didn't try any tags older than v3.0.0. Tested with go 1.18.1.
@mvdan
Copy link
Owner

mvdan commented Jun 30, 2022

Thanks, this does look like a bug. Happy to accept a patch with a test.

@mvdan mvdan changed the title panic on "let x/x" interp: panic on division by zero Jun 30, 2022
mvdan added a commit that referenced this issue Oct 20, 2022
@mvdan
expand: avoid panics on division by zero
bb7a399 
Arithm now returns one more type of error for this case.
Update the interpreter accordingly, and add tests.

The behavior we adopt follows what Bash does when not run interactively,
so start documenting that in the package godoc.

Also add a TODO as we should have some standard to tell whether errors
coming from the expand package or the user's handlers are fatal.
That is, whether they should exit the shell, and with what status code.

Fixes #892.
@mvdan mvdan mentioned this issue Oct 20, 2022
Merged
mvdan added a commit that referenced this issue Oct 21, 2022
@mvdan
expand: avoid panics on division by zero
e9df957 
Arithm now returns one more type of error for this case.
Update the interpreter accordingly, and add tests.

The behavior we adopt follows what Bash does when not run interactively,
so start documenting that in the package godoc.

Also add a TODO as we should have some standard to tell whether errors
coming from the expand package or the user's handlers are fatal.
That is, whether they should exit the shell, and with what status code.

While here, replace black and white with block and allow,
which are more self-explanatory.

Fixes #892.
mvdan added a commit that referenced this issue Oct 21, 2022
@mvdan
expand: avoid panics on division by zero
6ba49e2 
Arithm now returns one more type of error for this case.
Update the interpreter accordingly, and add tests.

The behavior we adopt follows what Bash does when not run interactively,
so start documenting that in the package godoc.

Also add a TODO as we should have some standard to tell whether errors
coming from the expand package or the user's handlers are fatal.
That is, whether they should exit the shell, and with what status code.

While here, replace black and white with block and allow,
which are more self-explanatory.

Fixes #892.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug good first issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

expand: avoid panics on division by zero mvdan/sh
2 participants
@dankegel @mvdan