Code review comment for lp:~epics-core/epics-base/devlib2mmio

Revision history for this message
Andrew Johnson (anj) wrote :

The sys{In|Out}{Byte|Word|Long}() routines are declared in sysLib.h. On most architectures the address argument is a ULONG (same as epicsUInt32), except on x86 where it's an int (but that doesn't seem to matter). I removed your redeclarations and added appropriate casts to the various read and write functions, and fixed the 68K check in the header file. The result built on all my architectures and the tests all passed on my mv2100, so I committed my changes.

Then I ran the tests on my mv6100. Here the 16- and 32-bit tests fail (running the exact same binary):

# 16-bit ops
not ok 3 - H16.u16==0x1234
ok 4 - nat_ioread16(&H16.bytes)==0x1234
not ok 5 - H16.u16==BE16
ok 6 - be_ioread16(&H16.bytes)==0x1234
not ok 7 - H16.u16==LE16
ok 8 - le_ioread16(&H16.bytes)==0x1234
# 32-bit ops
not ok 9 - H32.u32==0x12345678
ok 10 - nat_ioread32(&H32.bytes)==0x12345678
not ok 11 - H32.u32==BE32
ok 12 - be_ioread32(&H32.bytes)==0x12345678
not ok 13 - H32.u32==LE32
ok 14 - le_ioread32(&H32.bytes)==0x12345678

Ouch, that looks like a byte-swapping issue, so I made the mistake of looking more closely at the implementations of sysInWord(), sysOutWord() and their relatives.

Children, please stop whatever you are doing immediately, go outside, and run away as far and as fast as you can. Don't stop running until it starts getting dark. Then find someplace to hide, shut your eyes and go to sleep. Never come to this place again.

« Back to merge proposal