dohertj2/lmxopcua
1
0
Fork 0
Code Issues 1 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix(driver-abcip): resolve Medium code-review finding (Driver.AbCip-004)

Browse Source 
`ToDriverDataType` mapped LInt/ULInt to Int32 (truncation) and UDInt
to Int32 (negative wrap for values > Int32.MaxValue). DriverDataType
already carries Int64/UInt64/UInt32, so map each Logix 64-bit and
unsigned-32-bit type to the correct member. `DecodeValueAt` in
`LibplctagTagRuntime` updated to return uint/ulong for UDInt/ULInt
so the runtime value type agrees with the declared OPC UA type.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Joseph Doherty
2026-05-22 09:19:40 -04:00
parent 75580fb432
commit 1722c0328b
3 changed files with 19 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
code-reviews/Driver.AbCip/findings.md
View File

@@ -78,13 +78,13 @@
| Severity | Medium |
| Category | Correctness & logic bugs |
| Location | `AbCipDataType.cs:51-58`, `LibplctagTagRuntime.cs:47-49,53` |
| Status | Open |
| Status | Resolved |
**Description:** `ToDriverDataType` maps `LInt`/`ULInt` to `DriverDataType.Int32` (a TODO comment notes the gap) and `Dt` to `Int32`. But `LibplctagTagRuntime.DecodeValueAt` returns an actual `long` for `LInt`/`ULInt` (`_tag.GetInt64`, `(long)_tag.GetUInt64`). The address space is built declaring an Int32 node while the driver hands the server a boxed `long` `DataValueSnapshot.Value` at runtime: a mismatch between the declared OPC UA data type and the runtime value type. For `LInt` values exceeding Int32.MaxValue there is data loss if any consumer narrows it. `UDInt` is declared Int32 but decoded as `(int)_tag.GetUInt32`, so values above int.MaxValue wrap to negative.
**Recommendation:** Either add Int64/UInt32/UInt64 to `DriverDataType` and map correctly, or, until that lands, decode `LInt`/`ULInt` consistently with the declared `Int32` type (and document the truncation), and decode `UDInt` as a value that fits Int32 semantics. The declared type and the runtime value type must agree.
**Resolution:** _(open)_
**Resolution:** Resolved 2026-05-22 — `ToDriverDataType` now maps `LInt`→`Int64`, `ULInt`→`UInt64`, `UDInt`→`UInt32` (all already present in `DriverDataType`); `DecodeValueAt` updated to return `uint`/`ulong` for UDInt/ULInt respectively so the declared type and runtime value agree. The `(int)` and `(long)` casts that caused truncation/wrap are removed.
### Driver.AbCip-005

21
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDataType.cs
View File

@@ -40,22 +40,29 @@ public enum AbCipDataType
public static class AbCipDataTypeExtensions
{
/// <summary>
/// Map to the driver-agnostic type the server's address-space builder consumes. Unsigned
/// Logix types widen into signed equivalents until <c>DriverDataType</c> picks up unsigned
/// + 64-bit variants (Modbus has the same gap — see <c>ModbusDriver.MapDataType</c>
/// comment re: PR 25).
/// Map to the driver-agnostic type the server's address-space builder consumes.
/// <c>DriverDataType</c> carries Int64, UInt32, and UInt64 so each Logix type maps
/// to the widest correct signed/unsigned equivalent without silent truncation:
/// <list type="bullet">
/// <item>LInt (signed 64-bit) → Int64; ULInt (unsigned 64-bit) → UInt64.</item>
/// <item>UDInt (unsigned 32-bit) → UInt32 so values above Int32.MaxValue are not
/// wrapped to negative (Driver.AbCip-004).</item>
/// <item>USInt / UInt widen into Int32; they can never overflow it.</item>
/// </list>
/// </summary>
public static DriverDataType ToDriverDataType(this AbCipDataType t) => t switch
{
AbCipDataType.Bool => DriverDataType.Boolean,
AbCipDataType.SInt or AbCipDataType.Int or AbCipDataType.DInt => DriverDataType.Int32,
AbCipDataType.USInt or AbCipDataType.UInt or AbCipDataType.UDInt => DriverDataType.Int32,
AbCipDataType.LInt or AbCipDataType.ULInt => DriverDataType.Int32, // TODO: Int64 — matches Modbus gap
AbCipDataType.USInt or AbCipDataType.UInt => DriverDataType.Int32,
AbCipDataType.UDInt => DriverDataType.UInt32,
AbCipDataType.LInt => DriverDataType.Int64,
AbCipDataType.ULInt => DriverDataType.UInt64,
AbCipDataType.Real => DriverDataType.Float32,
AbCipDataType.LReal => DriverDataType.Float64,
AbCipDataType.String => DriverDataType.String,
AbCipDataType.Dt => DriverDataType.Int32, // epoch-seconds DINT
AbCipDataType.Structure => DriverDataType.String, // placeholder until UDT PR 6 introduces a structured kind
AbCipDataType.Structure => DriverDataType.String, // placeholder until UDT introduces a structured kind
_ => DriverDataType.Int32,
};
}

6
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagTagRuntime.cs
View File

@@ -44,9 +44,9 @@ internal sealed class LibplctagTagRuntime : IAbCipTagRuntime
AbCipDataType.Int => (int)_tag.GetInt16(offset),
AbCipDataType.UInt => (int)_tag.GetUInt16(offset),
AbCipDataType.DInt => _tag.GetInt32(offset),
AbCipDataType.UDInt => (int)_tag.GetUInt32(offset),
AbCipDataType.LInt => _tag.GetInt64(offset),
AbCipDataType.ULInt => (long)_tag.GetUInt64(offset),
AbCipDataType.UDInt => _tag.GetUInt32(offset), // UInt32 to match ToDriverDataType (Driver.AbCip-004)
AbCipDataType.LInt => _tag.GetInt64(offset), // Int64 to match ToDriverDataType (Driver.AbCip-004)
AbCipDataType.ULInt => _tag.GetUInt64(offset), // UInt64 to match ToDriverDataType (Driver.AbCip-004)
AbCipDataType.Real => _tag.GetFloat32(offset),
AbCipDataType.LReal => _tag.GetFloat64(offset),
AbCipDataType.String => _tag.GetString(offset),