drvAsynIPServerPort: bound the UDP read by the datagram, not by maxchars#233
Merged
Merged
Conversation
readIt() copied from the staged datagram with a loop bounded by the caller's
buffer size instead of by the number of bytes actually received, and then
advanced UDPbufferPos by a different amount again. Three consequences:
- The copy was never bounded by UDPbufferSize. Reading a 10-byte datagram
with a 256-byte buffer copied 255 bytes -- 10 real ones followed by 245
bytes of the previous datagram's tail, or, on the first read after connect,
of never-written malloc() memory -- and reported all 255 through
*nbytesTransferred. Any device support that trusts the transfer count
rather than scanning for an EOS terminator parses that as received data.
- The loop copied maxchars-1 bytes but UDPbufferPos advanced by maxchars,
so one byte of every datagram was skipped.
- maxchars == 1 copied nothing yet still consumed a byte, so a byte-at-a-time
reader never made progress.
UDPbufferSize is the datagram length returned by recvfrom(); it was read only
after the copy, to pick the EOM reason, and never bounded the loop.
Treat the staged datagram as occupying [UDPbufferPos, UDPbufferSize): deliver
min(remaining, maxchars) bytes and advance UDPbufferPos by exactly what was
delivered. The datagram is then exhausted precisely when UDPbufferPos reaches
UDPbufferSize, which is the existing test that picks ASYN_EOM_END over
ASYN_EOM_CNT, so the EOM and buffer-release semantics are unchanged. A read
that exactly fills the caller's buffer now leaves thisRead == maxchars, and
the trailing "if there is room add a null byte" test correctly skips the null
and reports ASYN_EOM_CNT.
Verified against a real UDP port on the loopback interface, reading through
asynOctetSyncIO:
before:
200-byte datagram, read with maxchars=4096 -> reported 4095 bytes
2-byte datagram "HI", read with maxchars=64 -> reported 63 bytes:
"HICRET-PASSWORD-SECRET-PASSWORD-SECRET-PASSWORD-SECRET-PASSWORD"
(61 bytes of the preceding datagram, reported as received)
"ABCDEFGHIJ" read 4 bytes at a time -> "ABCEFGIJS"
"XY" read 1 byte at a time -> "" (0 bytes, both consumed)
after:
200-byte datagram, read with maxchars=4096 -> 200 bytes
2-byte datagram "HI", read with maxchars=64 -> 2 bytes, "HI"
"ABCDEFGHIJ" read 4 bytes at a time -> "ABCDEFGHIJ"
"XY" read 1 byte at a time -> "XY"
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
readItin the UDP path copies from the staged datagram buffer using the caller'smaxcharsas the bound rather than the number of bytes actually received (UDPbufferSize). Three faults follow from that one mistake:mallocmemory. All 255 are reported as received via*nbytesTransfered.maxchars - 1bytes butUDPbufferPosadvances bymaxchars, so one byte of every datagram is silently dropped.maxchars == 1deadlock. The loop body never runs,thisRead = 0, but the position still advances by 1 — a byte-at-a-time reader makes no progress.Fixed structurally: the staged datagram occupies
[UDPbufferPos, UDPbufferSize); delivermin(remaining, maxchars)and advance by exactly what was delivered. The existingUDPbufferSize <= UDPbufferPostest then selects EOM_END/EOM_CNT unchanged.Verified against a real UDP port on loopback read via asynOctetSyncIO:
"HI", maxchars=64"HI""ABCDEFGHIJ"split over reads"ABCEFGIJ…"(dropped bytes)"ABCDEFGHIJ""XY", maxchars=1""(no progress)"XY"