From fa7d72d342cc648f87f7d87a0b21823818e64bd3 Mon Sep 17 00:00:00 2001 From: Rob Shakir Date: Tue, 9 Jul 2024 20:58:09 -0700 Subject: [PATCH] Fix an age-old bug with `FromFloat` handling of negatives. * (M) pkg/yang/types_builtin(_test)?.go - This change squashes a bug that has existed for a long time in `FromFloat`, but only shows up on arm64. I finally have an arm64 machine and so found the issue. It was originally reported in https://github.com/openconfig/ygot/issues/766. The issue is that uint64 of a negative float64 is undefined -- and on arm64 on darwin returns 0, which made some test cases fail only on this architecture. --- pkg/yang/types_builtin.go | 5 ++-- pkg/yang/types_builtin_test.go | 48 ++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/pkg/yang/types_builtin.go b/pkg/yang/types_builtin.go index 108ecaa..95a4e84 100644 --- a/pkg/yang/types_builtin.go +++ b/pkg/yang/types_builtin.go @@ -328,18 +328,17 @@ func FromFloat(f float64) Number { } } - // Per RFC7950/6020, fraction-digits must be at least 1. fracDig := uint8(1) f *= 10.0 for ; Frac(f) != 0.0 && fracDig <= MaxFractionDigits; fracDig++ { f *= 10.0 } - v := uint64(f) negative := false if f < 0 { negative = true - v = -v + f = -f } + v := uint64(f) return Number{Negative: negative, Value: v, FractionDigits: fracDig} } diff --git a/pkg/yang/types_builtin_test.go b/pkg/yang/types_builtin_test.go index 1d92a8e..0bdf5d9 100644 --- a/pkg/yang/types_builtin_test.go +++ b/pkg/yang/types_builtin_test.go @@ -55,6 +55,54 @@ func Rf(a, b int64, fracDig uint8) YRange { return YRange{n1, n2} } +func TestFromFloat(t *testing.T) { + tests := []struct { + desc string + in float64 + want Number + }{{ + desc: "positive - no decimals", + in: 10.0, + want: Number{ + Negative: false, + Value: 10, + FractionDigits: 0, + }, + }, { + desc: "positive - decimals", + in: 10.15, + want: Number{ + Negative: false, + Value: 1015, + FractionDigits: 2, + }, + }, { + desc: "negative - no decimals", + in: -10.0, + want: Number{ + Negative: true, + Value: 10, + FractionDigits: 0, + }, + }, { + desc: "negative - decimals", + in: -10.15, + want: Number{ + Negative: true, + Value: 1015, + FractionDigits: 2, + }, + }} + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + if got := FromFloat(tt.in); !cmp.Equal(got, tt.want) { + t.Fatalf("FromFloat(%v): did not get expected value, got: %+v, want: %+v", tt.in, got, tt.want) + } + }) + } +} + func TestNumberInt(t *testing.T) { tests := []struct { desc string