Repository navigation
Conversation
Add expected_size field to test patterns to properly validate cases where the requested read size exceeds the actual file size. This allows testing the server's behavior when clients request more data than is available. When a read request is larger than the file, the server correctly returns data up to EOF. The test framework now distinguishes between this expected scenario and genuine read failures. Signed-off-by: Oleksij Rempel <linux@rempel-privat.de>
The cmd_exit() function used -EINTR to signal a clean exit from the interactive mode. This is semantically incorrect because EINTR is an errno value meant for system call interruptions, not application-level control flow. Stop using -EINTR for cmd_exit(). Introduce ISOBUSFS_CLI_RET_EXIT (a positive return code) to distinguish control flow from actual errors. Map this to exit code 0 in main(), keeping negative errno values for real errors. This makes the exit path explicit and prevents confusion with actual interrupted system calls. Signed-off-by: Oleksij Rempel <linux@rempel-privat.de>
Fix directory operations to comply with ISO 11783-13:2021 (sections
C.3.4.2 and C.3.5.2), which specifies that directory operations differ
significantly from standard file operations:
- The Count parameter in a Read File request represents the number of
directory entries to read, not the number of bytes.
- The Offset parameter in a Seek File request represents the logical
entry index, not a byte offset.
The previous implementation treated directories strictly as files,
using byte-based offsets and counts. This resulted in incorrect seeking
behavior and protocol violations when listing directories.
Align the implementation with the standard by:
1. Server side:
- Introduce isobusfs_srv_dir_entry_visible() to consistently
filter out invalid (unreadable, hidden, oversized) entries. This
ensures that entry indices remain stable.
- Implement isobusfs_srv_dir_skip_entries() to advance the
directory stream by logical visible entries rather than bytes.
- Update the Read File handler to interpret count as the
maximum number of entries and return the number of entries read
in the response header.
- Update the Seek File handler to seek by entry index.
2. Client side:
- Calculate the request count based on the number of minimal-size
entries that fit into the maximum data length.
- Interpret the response count as the number of entries received
rather than the byte length of the payload.
Signed-off-by: Oleksij Rempel <linux@rempel-privat.de>
marckleinebudde
requested changes
May 26, 2026
| remaining_size = ((int64_t)tp->offset + tp->read_size) - | ||
| ((int64_t)priv->read_offset + priv->read_data_len); | ||
| pr_debug("remaining_size: %lld, read_offset: %zu, read_data_len: %zu, test read size: %u, test offset %u", | ||
| (long long)remaining_size, priv->read_offset, priv->read_data_len, |
Member
There was a problem hiding this comment.
If there isn'a max file size defined by the protocol, I suggest to use size_t in tp->offset, tp->read_size and friends.
olerem
commented
May 26, 2026
| (priv->read_offset + priv->read_data_len); | ||
| pr_debug("remaining_size: %zd, read_offset: %zu, read_data_len: %zu, test read size: %zu, test offset %zu", | ||
| remaining_size, priv->read_offset, priv->read_data_len, | ||
| remaining_size = ((int64_t)tp->offset + tp->read_size) - |
Contributor
Author
There was a problem hiding this comment.
I need to verify maximal sizes supported by protocol..
| uint32_t actual_sum, expected_sum; | ||
| struct timespec current_time; | ||
| ssize_t remaining_size, read_size; | ||
| int64_t remaining_size; |
Contributor
Author
There was a problem hiding this comment.
Do we need here signed variable?
marckleinebudde
force-pushed
the
isobusfs-clt-srv-transfer-fixes
branch
from
October 14, 2026 09:28
67e0b97 to
61eb881
Compare
hartkopp
pushed a commit
to hartkopp/can-utils
that referenced
this pull request
Oct 14, 2026
Add expected_size field to test patterns to properly validate cases where the requested read size exceeds the actual file size. This allows testing the server's behavior when clients request more data than is available. When a read request is larger than the file, the server correctly returns data up to EOF. The test framework now distinguishes between this expected scenario and genuine read failures. Signed-off-by: Oleksij Rempel <linux@rempel-privat.de> [socketcan: addressed review comments from PR linux-can#629: - keep uint32_t instead of size_t for offset/read_size/expected_size: ISO 11783-13 limits the file offset (Seek File) and the file size (directory entry) to 32 bit, so uint32_t matches the protocol. The sum offset + read_size can exceed 32 bit (see UINT32_MAX pattern) and is therefore calculated in int64_t. size_t would not help here as it is only 32 bit wide on 32 bit targets. - remaining_size is a signed int64_t: the signed type is needed to detect the 'remaining_size < 0' case (server returned more data than requested) and int64_t holds the 33 bit sum on all platforms. - fix printf format for the uint32_t read_offset (%zu -> %u)] Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Member
|
Superseded by #635 |
olerem
added a commit
that referenced
this pull request
Oct 14, 2026
The first three patches are from PR #629 that was stuck due to the review process. The review questions of commit #629 (review) have been addressed in this PR (see commit message) The most important fix that has an impact on the data structures on the CAN bus is the last one ("isobusfs: pack the protocol structures"). "The isobusfs protocol structures describe the message layout defined in ISO 11783-13 but were not packed. The compiler added padding bytes in front of 16/32 bit values which follow an odd number of bytes, e.g. the Read File Request (8 bytes according to C.3.5.2) had a size of 10 bytes and the count value at the wrong position. Server and client of the can-utils used the same structures and understood each other but the messages were not compatible to other ISOBUS devices." So this has to be checked and reviewed carefully as it breaks the former protocol of can-utils (but likely fits the rest of the world).
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
3 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.
This patch series address the directory listing implementation.