cast icon indicating copy to clipboard operation
cast copied to clipboard

Add overflow check

Open lunemec opened this issue 6 years ago • 3 comments

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 avatar Jul 12 '18 08:07 lunemec

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

lunemec avatar Jul 12 '18 08:07 lunemec

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

bep avatar Oct 24 '18 07:10 bep

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

lunemec avatar Oct 24 '18 12:10 lunemec