Skip to content

Commit fa7d72d

Browse files
committed
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 openconfig/ygot#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.
1 parent dfbf7ba commit fa7d72d

File tree

2 files changed

+50
-3
lines changed

2 files changed

+50
-3
lines changed

pkg/yang/types_builtin.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -328,18 +328,17 @@ func FromFloat(f float64) Number {
328328
}
329329
}
330330

331-
// Per RFC7950/6020, fraction-digits must be at least 1.
332331
fracDig := uint8(1)
333332
f *= 10.0
334333
for ; Frac(f) != 0.0 && fracDig <= MaxFractionDigits; fracDig++ {
335334
f *= 10.0
336335
}
337-
v := uint64(f)
338336
negative := false
339337
if f < 0 {
340338
negative = true
341-
v = -v
339+
f = -f
342340
}
341+
v := uint64(f)
343342

344343
return Number{Negative: negative, Value: v, FractionDigits: fracDig}
345344
}

pkg/yang/types_builtin_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,54 @@ func Rf(a, b int64, fracDig uint8) YRange {
5555
return YRange{n1, n2}
5656
}
5757

58+
func TestFromFloat(t *testing.T) {
59+
tests := []struct {
60+
desc string
61+
in float64
62+
want Number
63+
}{{
64+
desc: "positive - no decimals",
65+
in: 10.0,
66+
want: Number{
67+
Negative: false,
68+
Value: 10,
69+
FractionDigits: 0,
70+
},
71+
}, {
72+
desc: "positive - decimals",
73+
in: 10.15,
74+
want: Number{
75+
Negative: false,
76+
Value: 1015,
77+
FractionDigits: 2,
78+
},
79+
}, {
80+
desc: "negative - no decimals",
81+
in: -10.0,
82+
want: Number{
83+
Negative: true,
84+
Value: 10,
85+
FractionDigits: 0,
86+
},
87+
}, {
88+
desc: "negative - decimals",
89+
in: -10.15,
90+
want: Number{
91+
Negative: true,
92+
Value: 1015,
93+
FractionDigits: 2,
94+
},
95+
}}
96+
97+
for _, tt := range tests {
98+
t.Run(tt.desc, func(t *testing.T) {
99+
if got := FromFloat(tt.in); !cmp.Equal(got, tt.want) {
100+
t.Fatalf("FromFloat(%v): did not get expected value, got: %+v, want: %+v", tt.in, got, tt.want)
101+
}
102+
})
103+
}
104+
}
105+
58106
func TestNumberInt(t *testing.T) {
59107
tests := []struct {
60108
desc string

0 commit comments

Comments
 (0)