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

Add overflow check #63

Open
lunemec opened this issue Jul 12, 2018 · 3 comments
Open

Add overflow check #63

lunemec opened this issue Jul 12, 2018 · 3 comments

Comments

@lunemec
Copy link

lunemec commented Jul 12, 2018

You clam your library to be "safe casting from one type to another in Go", but in the code there are no checks for overflow when casting integer types:

switch s := i.(type) {
	case int:
		return int16(s), nil  // <-- this may overflow
	case int64:
		return int16(s), nil  // <-- this may overflow
	case int32:
		return int16(s), nil
	case int16:
		return s, nil
	case int8:
		return int16(s), nil
	case uint:
		return int16(s), nil   // <-- this may overflow
	case uint64:
		return int16(s), nil   // <-- this may overflow
	case uint32:
		return int16(s), nil   // <-- this may overflow
	case uint16:
		return int16(s), nil   // <-- this may overflow
	case uint8:
		return int16(s), nil
	case float64:
		return int16(s), nil   // <-- this may overflow
	case float32:
		return int16(s), nil  // <-- this may overflow

Adding bounds check like this: https://play.golang.org/p/Jgu2NYg1qi3 would solve the issue.

@lunemec
Copy link
Author

lunemec commented Jul 12, 2018

I'd gladly write the addition, if you promise not to leave the pull request hanging for months before merge 😉

@bep
Copy link
Collaborator

bep commented Oct 24, 2018

You clam your library to be "safe casting from one type to another in Go"

If we can agree that this isn't "my library" and I have made no such promise (:-)), I PR for this would be appreciated. I will merge it.

But I'm not sure if we should return error or panic.

Most people using this are probably just doing ToInt(234) and will not receive any overflow error. So possibly a panic?

/cc @moorereason @spf13

@lunemec
Copy link
Author

lunemec commented Oct 24, 2018

I created a small library that does just the overflow checking: https://github.com/lunemec/as

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

No branches or pull requests

2 participants