FDio / govpp

Go toolset for the VPP.
Apache License 2.0
193 stars 82 forks source link

cannot parse high u64 default values in the API files #118

Open hardeker opened 1 year ago

hardeker commented 1 year ago

As Govpp doesn't know in advance the exact template of the json it parses, it uses a generic parsing with json.Unmarshal here However, when unmarshalling generic json in Go, numbers are casted in float64. Now, if the Json contain uint64 numbers, we lose precision casting them as float64.

As we allow u64 types in the VPP API, we may end up with a wrong value after parsing the API. And casting float64 into uint64 depends on the hardware implementation. In Govpp, the conversion happens here Let's take a VPP API exmaple:

 [
    "u64",
    "value",
    {
        "default": 18446744073709551615
    }
],

If we generate Go bindings with this Json, we get:

hardeker commented 1 year ago

I will have a look at other VPP bindings generators to see how they react to that.

ondrej-fabry commented 1 year ago

I have played around a bit with this and so far it seems the library we are using to process JSON does not seem to be flexible for parsing numbers. There is MustGetUint64 method, but it does not return the correct number, most likely due incorrect conversions.

I also tried standard encoding/json to parse number with max uint64 value and I was able to properly parse it only when decoding the value directly into uint64 variable or decoding into json.Number (practically a string) and then using strconv.ParseUint to convert it into uint64, because the json.Number only has Float64 and Int64 methods for quick conversion.

I don't want to spend much more time than I already did, but I plan to come back to this issue later after I am done with other more urgent matters. The default option for field is not used anywhere in the GoVPP beside being set as struct tag in the generated bindings and also there is no case that I know of in the official VPP API that has the default set to max uint64.

Anyone is welcome to take a shot and open PR if you find a fix for this.

hardeker commented 1 year ago

I just had a look at the Python bindings. The json library commonly used in Python does differentiate between int and float. If the number contains a dot, it is considered as a float, otherwise, it is considered as an int. Thus, the problem does not appear in Python.