Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

calc_blklen violates specification section "9.5 Segmented Streaming" #5

Open
Parakleta opened this issue Jun 9, 2024 · 2 comments
Open

Comments

@Parakleta
Copy link

The relevant section reads:

9.5 Segmented Streaming

If the receiver cannot overlap serial and disk I/O, it uses the
ZRINIT frame to specify a buffer length which the sender will not
overflow. The sending program sends a ZCRCW data subpacket and waits
for a ZACK header before sending the next segment of the file.

If the sending program supports reverse data stream sampling or
interrupt, error recovery will be faster (on average) than a protocol
(such as YMODEM) that sends large blocks.

A sufficiently large receiving buffer allows throughput to closely
approach that of full streaming. For example, 16kb segmented
streaming adds about 3 per cent to full streaming ZMODEM file
transfer times when the round trip delay is five seconds.

If a maximum buffer length is provided in the ZRINIT frame this is read by lsz and stored in the Rxbuflen and blklen variables, but then later when calc_blklen is called the blklen value is overridden (set to 1024) and the Rxbuflen value is never again consulted.

This is problematic for my use case because I trying to use ZMODEM to program small embedded devices which have limited RAM and a specific flash block size which must be programmed. I am trying to set the buffer length to one flash page so that I can read it in, checksum it, and then write it to flash before accepting the next subpacket.

I think the solution to this is to set the start_blklen and max_blklen variables as well when Rxbuflen is set, if it is smaller than Tframelen, exactly the same way that these are set when it is.

@Parakleta
Copy link
Author

As an addition to this, the following section of code makes it impossible to test this behaviour using pipes to a simulated receiver as it overrides the Rxbuflen value:

lrzsz/src/lsz.c

Lines 1260 to 1263 in 8cb2a6a

fstat(0, &f);
if (! (S_ISCHR(f.st_mode))) {
Rxbuflen = MAX_BLOCK;
}

@Parakleta
Copy link
Author

Further, the commit fbdffdb breaks the required behaviour further by preventing a ZCRCW request from being made after each Rxbuflen sized block. The comment stating that the newcnt variable was never updated was incorrect. The following code

lrzsz/src/lsz.c

Lines 1593 to 1594 in 8cb2a6a

if (e == ZCRCW)
goto waitack;
jumps (I know, the goto's are atrocious) to above where newcnt was set, thus updating it again to allow another full block.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant