Add TOML unmarshal implementation #8
Conversation
bytes.go
Outdated
| return err | ||
| } | ||
| case int, int8, int16, int32, int64: | ||
| *b = Base2Bytes(reflect.ValueOf(v).Int()) |
There was a problem hiding this comment.
I think it can fail with an overflow error if v > MaxInt32. Could you please add a test for int64 value that is > max int32
There was a problem hiding this comment.
Also I'd prefer not to use reflect unless necessary, which it doesn't appear to be.
There was a problem hiding this comment.
Updated. Thank you for review.
Talking about overflow. The following code in parser checks for overflows:
if e, ok := err.(*strconv.NumError); ok &&
e.Err == strconv.ErrRange {
p.panicf("Integer '%s' is out of the range of 64-bit "+
"signed integers.", it.val)https://github.com/BurntSushi/toml/blob/a368813c5e648fee92e5f6c30e3944ff9d5e8895/parse.go#L196
Also in TOML spec ints are always int64.
| @@ -25,6 +30,8 @@ var ( | |||
| oldBytesUnitMap = MakeUnitMap("B", "B", 1024) | |||
| ) | |||
|
|
|||
| var ErrIncorrectType = errors.New("not a string or int64") | |||
There was a problem hiding this comment.
can be int, int8, int16, int32 as well
There was a problem hiding this comment.
It can be only int64 or string.
TOML spec says that all ints are int64:
64 bit (signed long) range expected (−9,223,372,036,854,775,808 to 9,223,372,036,854,775,807).
There was a problem hiding this comment.
may be you should rename error? it is toml specific incorrect type error, but not package-wide incorrect type error.
3edde86 to
6a0232f
Compare
|
Hi, the Prometheus/Thanos use this lib, also need to deal with yaml. I found a problem the marshalled data can not be unmarshal. |
No description provided.