Link initialization from hex constants is broken

[Ralph Lange]

From Andrew Starrit <andrew.starritt AT synchrotron.org.au>:

I recently upgraded many of my IOCs from base R3.14.12.5 to R3.15.5. I have noticed a
difference regarding literal hexadecimal numbers in INP fields as illustrated by the
following example (the transition was otherwise painless):

record (ai, "AI:HEX") {
    field (SCAN, ".1 second")
    field (INP, "0x0200")
}

record (longin, "LI:HEX") {
    field (SCAN, ".1 second")
    field (INP, "0x0200")
}

record (aSub, "AS:HEX") {
    field (SCAN, ".1 second")
    field (INPA, "0x0200")
    field (FTA, "LONG")
    field (INPB, "0x0200")
    field (FTB, "DOUBLE")
}

For EPICS R3.14.12.5 I get:
$ caget AI:HEX LI:HEX AS:HEX.A AS:HEX.B
AI:HEX 512
LI:HEX 512
AS:HEX.A 512
AS:HEX.B 512

while for EPICS R3.15.5 I now get:
$ caget AI:HEX LI:HEX AS:HEX.A AS:HEX.B
AI:HEX 512
LI:HEX 0
AS:HEX.A 0
AS:HEX.B 512

[Ralph Lange (ralph-lange)]

EPICS 3.16.1 shows:
$ caget AI:HEX LI:HEX AS:HEX.A AS:HEX.B
AI:HEX 512
LI:HEX 0
AS:HEX.A 0
AS:HEX.B 0

Which is yet different.

[Ralph Lange (ralph-lange)]

Reason seems to lie below recGblInitConstantLink():

3.14 directly calls strtol()/strtoul() for longs. This honours hex constants.
3.15ff. calls dbLoadLink() -> dbConstLoadLink() -> dbFastConvertRoutine[STRING][LONG] which is putStringLong() that calls sscanf() using "%ld". Which does not honour hex constants.

3.15ff. added a set of parsing functions epicsParseXxx() in epicsStdlib.c, which are thoroughly tested. Could these be used?

[Andrew Johnson (anj)]

The IOC sees the link as a constant, but can't convert the hex as you say:

epics> dbpr LI:HEX 1
...
HOPR: 0 HSV: NO_ALARM HYST: 0 INP:CONSTANT 0x0200
...
UDF: 0 UDFS: INVALID VAL: 0

@Ralph you're right up until the dbFastConvertRoutine[][] part, but putStringLong() is from dbConvert.c; the Fast convert routines are found in dbFastLinkConv.c and in the above case it is cvt_st_l() which handles the conversion. The 3.15 version calls strtol(from, &end, 10) where the explicit base=10 argument tells it not to allow hex numbers at all. As long as at least one valid digit is seen cvt_st_l() does not return an error (because that's the way it has always worked).

In Base-3.16.1 both dbConvert.c and dbFastLinkConv.c have already been converted to use the new epicsParseXxx() routines, so here cvt_st_l() calls epicsParseInt32(from, to, 10, &end) which is also passing base=10 with the same effect.

The fix has to be to provide another conversion table for dbConstLoadLink() [which became dbConstLoadScalar() in 3.16.1] that does allow hex numbers.

Base-3.16.1 also accepts arrays [1,2,3] as constants, and there the number parsing is done by the JSON parser; the JSON spec doesn't allow hex numbers though, so no hex arrays. However there is a proposed JSON5 extension (see http://json5.org/ for details) which looks a lot more like my "relaxed JSON" and does allow hex. I would be willing to switch to that standard; there may even be patches available for the yajl parser.

[Andrew Johnson (anj)]

I have a fix (attached) for 3.15 and a different one (not included) for the ai and longin (i.e. scalar) cases in 3.16, but in 3.16 the aSub input links can initialize arrays, and there is a new routine dbLoadLinkArray() for doing that. This calls the JSON parser for the conversions, hence no hex as I discussed previously unless we switch to the JSON5 syntax. I do want to do that, but it isn't a simple change and will depend on my yajl-2.1 branch.

[Ralph Lange (ralph-lange)]

Excellent! Thanks a lot Andrew - that was quick.
Do you want to post this (and a link to the patch) on the tech-talk thread?

[mdavidsaver (mdavidsaver)]

Any chance of adding some regression tests?

[Andrew Johnson (anj)]

Sure, see attached. I won't include these changes in the Known Problems patch though.

The last 2 tests will fail in 3.16 until we switch to JSON5, so I will need to mark them as TODO when I merge up the commit.

[Andrew Johnson (anj)]

As an aside I just noticed that hex constants are not currently permitted in 3.14 when the field being initialized is UCHAR, USHORT or ULONG; the first two are converted using sscanf("%u"), the latter using strtoul() with base=10. That seems exactly the wrong way round to me, but anyway...

From my updated regression test database:

record(aSub, "as1") {
  field(INPA, 0x10)
  field(FTA, "CHAR")
  field(INPB, 0x10)
  field(FTB, "UCHAR")
  field(INPC, 0x10)
  field(FTC, "SHORT")
  field(INPD, 0x10)
  field(FTD, "USHORT")
  field(INPE, 0x10)
  field(FTE, "LONG")
  field(INPF, 0x10)
  field(FTF, "ULONG")
  field(INPG, 0x10)
  field(FTG, "FLOAT")
  field(INPH, 0x10)
  field(FTH, "DOUBLE")
}

On 3.14.12.6 this gives:

tux% caget as1.A as1.B as1.C as1.D as1.E as1.F as1.G as1.H
as1.A 16
as1.B 0
as1.C 16
as1.D 0
as1.E 16
as1.F 0
as1.G 16
as1.H 16

[Andrew Johnson (anj)]

Fix and tests committed to the 3.15 branch and merged up into 3.16 with TODOs for the array tests that fail.
Patch published on the 3.15 Known Problems page.

[Andrew Johnson (anj)]

Fixing the hex array issues in the fix for lp: #171445