ast icon indicating copy to clipboard operation
ast copied to clipboard

Several #define NV_ numeric types are suspect

Open krader1961 opened this issue 6 years ago • 1 comments

The symbols NV_INT32, NV_INT64 and NV_UINT16 are used to set the flags of a struct Namval but are not used to test if those attributes are set. That is highly suspicious. NV_INT32 is just an alias for NV_INTEGER and the other two set that bit and one other bit. Which suggests that it's only the setting of the NV_INTEGER bit that matters. But it is also possible that the places which test whether the value is one of those int types is also testing, as one example, that NV_INTEGER and NV_LONG being set using two distinct tests rather than testing if NV_INT64 is set. The two approaches are equivalent but highly misleading and will break if NV_INT64 ever becomes a distinct bit in the value type bit-mask.

Similarly there is exactly one use of NV_FLOAT:

https://github.com/att/ast/blob/70c3d35b0d22ac42e0a3c51b7df8abed7a70ca68/src/cmd/ksh93/bltins/typeset.c#L1275-L1278

This is also highly suspect and implies the correct test at that point is whether NV_DOUBLE is present.

krader1961 avatar Apr 26 '19 02:04 krader1961

As expected there is at least one use of NV_INTEGER | NV_LONG that should be NV_INT64. In src/cmd/ksh93/data/options.csrc/cmd/ksh93/data/options.c:

https://github.com/att/ast/blob/f68b4029d6cd6faa1d6463eaa5198f240dfd0d54/src/cmd/ksh93/data/options.c#L160

krader1961 avatar Jun 01 '19 05:06 krader1961