Skip to content

Isobusfs clt srv transfer fixes #629

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.

Sign up for GitHub

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

Open
olerem wants to merge 3 commits into
base: master
Choose a base branch
from
+330 −95
Open

Isobusfs clt srv transfer fixes #629

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5a68196
isobusfs: selftest: Handle read requests larger than file size
olerem Jan 4, 2026
a5bd791
isobusfs: treat interactive exit as control flow, not error
olerem Jan 6, 2026
61eb881
isobusfs: fix directory read and seek semantics to match ISO 11783-13
olerem Jan 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions isobusfs/isobusfs_cli.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,10 @@ int main(int argc, char *argv[])

while (1) {
ret = isobusfs_cli_process_events_and_tasks(priv);
if (ret == ISOBUSFS_CLI_RET_EXIT) {
ret = 0;
break;
}
if (ret)
break;
}
Expand Down
3 changes: 3 additions & 0 deletions isobusfs/isobusfs_cli.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
#define ISOBUSFS_CLI_MAX_EPOLL_EVENTS 10
#define ISOBUSFS_CLI_DEFAULT_WAIT_TIMEOUT_MS 1000 /* ms */

/* internal return codes, not errno values */
#define ISOBUSFS_CLI_RET_EXIT 1

enum isobusfs_cli_state {
ISOBUSFS_CLI_STATE_CONNECTING,
ISOBUSFS_CLI_STATE_IDLE,
Expand Down
105 changes: 76 additions & 29 deletions isobusfs/isobusfs_cli_int.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@
#define MAX_COMMAND_LENGTH 256

#define MAX_DISPLAY_FILENAME_LENGTH 100
/*
* ISO 11783-13:2021 B.21 minimal directory entry payload size in bytes:
* 1 (name length) + 1 (min name byte) + 1 (attributes) +
* 2 (date) + 2 (time) + 4 (size).
*/
#define ISOBUSFS_MIN_DIR_ENTRY_SIZE (1 + 1 + 1 + 2 + 2 + 4)

struct command_mapping {
const char *command;
Expand Down Expand Up @@ -66,8 +72,8 @@ static int cmd_help(struct isobusfs_priv *priv, const char *options)
static int cmd_exit(struct isobusfs_priv *priv, const char *options)
{
pr_int("exit interactive mode\n");
/* Return -EINTR to indicate the program should exit */
return -EINTR;

return ISOBUSFS_CLI_RET_EXIT;
}

static int cmd_dmesg(struct isobusfs_priv *priv, const char *options)
Expand Down Expand Up @@ -510,8 +516,12 @@ isobusfs_cli_ls_handle_open_dir_sent(struct isobusfs_priv *priv,

ctx->handle = res->handle;

ctx->offset = 0;
ctx->entry_count = 0;

ret = isobusfs_cli_send_and_register_fa_sf_event(priv, ctx->handle,
0, ctx->entry_count,
ISOBUSFS_FA_SEEK_SET,
ctx->offset,
cb, ctx);
if (ret)
pr_int("Failed to send seek file request: %i\n", ret);
Expand All @@ -530,7 +540,7 @@ isobusfs_cli_ls_handle_seek_dir_sent(struct isobusfs_priv *priv,
{
isobusfs_event_callback cb = isobusfs_cli_ls_event_callback;
struct isobusfs_fa_seekf_res *res =
(struct isobusfs_fa_seekf_res *)msg;
(struct isobusfs_fa_seekf_res *)msg->buf;
uint16_t count;
int ret;

Expand All @@ -543,8 +553,10 @@ isobusfs_cli_ls_handle_seek_dir_sent(struct isobusfs_priv *priv,
goto error;
}

/* set max possible number fitting in to 16bits */
count = UINT16_MAX;
/* ISO 11783-13:2021 C.3.5.2: count is number of directory entries. */
count = ISOBUSFS_MAX_DATA_LENGH / ISOBUSFS_MIN_DIR_ENTRY_SIZE;
if (!count)
count = 1;
ctx->request_count = count;

ret = isobusfs_cli_send_and_register_fa_rf_event(priv, ctx->handle,
Expand Down Expand Up @@ -616,8 +628,8 @@ static bool isobusfs_cli_extract_directory_entry(const uint8_t *buffer,
uint16_t *file_time,
uint32_t *file_size)
{
size_t entry_total_len, copy_len;
uint8_t filename_length;
size_t entry_total_len;

if (*pos + 2 > buffer_length) {
pr_int("Error: Incomplete data in buffer\n");
Expand All @@ -633,8 +645,14 @@ static bool isobusfs_cli_extract_directory_entry(const uint8_t *buffer,
}

(*pos)++;
strncpy(filename, (const char *)buffer + *pos, filename_length);
filename[filename_length] = '\0';

if (filename_length > ISOBUSFS_MAX_DIR_ENTRY_NAME_LENGTH)
copy_len = ISOBUSFS_MAX_DIR_ENTRY_NAME_LENGTH;
else
copy_len = filename_length;

strncpy(filename, (const char *)buffer + *pos, copy_len);
filename[copy_len] = '\0';
*pos += filename_length;
if (filename_length > MAX_DISPLAY_FILENAME_LENGTH) {
/* Truncate the filename and replace the last character
Expand Down Expand Up @@ -688,15 +706,17 @@ isobusfs_cli_print_directory_entry(struct isobusfs_cli_ls_context *ctx,
static void
isobusfs_cli_print_directory_entries(struct isobusfs_cli_ls_context *ctx,
const uint8_t *buffer,
size_t buffer_length)
size_t buffer_length,
uint16_t max_entries)
{
char filename[ISOBUSFS_MAX_DIR_ENTRY_NAME_LENGTH + 1];
uint16_t file_date, file_time;
uint32_t file_size;
uint8_t attributes;
size_t pos = 0;
uint16_t entries = 0;

while (pos < buffer_length) {
while (pos < buffer_length && entries < max_entries) {
if (!isobusfs_cli_extract_directory_entry(buffer, buffer_length,
&pos, filename,
&attributes,
Expand All @@ -709,6 +729,7 @@ isobusfs_cli_print_directory_entries(struct isobusfs_cli_ls_context *ctx,
file_date, file_time,
file_size);
ctx->entry_count++;
entries++;
}
}

Expand All @@ -721,26 +742,42 @@ isobusfs_cli_ls_handle_read_dir_sent(struct isobusfs_priv *priv,
(struct isobusfs_read_file_response *)msg->buf;
size_t buffer_length = msg->len - sizeof(*res);
isobusfs_event_callback cb;
size_t entries_before;
size_t entries_in_message;
uint16_t count;
int ret;

pr_debug("< rx: Read File Response. Error code: %i", res->error_code);

if (isobusfs_cli_int_is_error(priv, 0, res->error_code, res->tan))
goto error;

count = le16toh(res->count);
if (count && count != buffer_length) {
pr_int("Buffer length mismatch: %u != %zu\n", count,
buffer_length);
goto error;
if (count && buffer_length) {
entries_before = ctx->entry_count;
isobusfs_cli_print_directory_entries(ctx, res->data,
buffer_length, count);
entries_in_message = ctx->entry_count - entries_before;
} else {
entries_in_message = 0;
}

if (count)
isobusfs_cli_print_directory_entries(ctx, res->data,
buffer_length);
if (count != entries_in_message) {
pr_warn("Directory entry count mismatch: server sent %u, parsed %zu\n",
count, entries_in_message);
/* Continue processing if we parsed at least some entries.
* Strict validation would reject valid responses if parsing
* fails partway through due to corruption.
*/
if (entries_in_message == 0 && count > 0) {
pr_int("Error: Failed to parse any directory entries\n");
goto error;
}
}

cb = isobusfs_cli_ls_event_callback;
if (count) {

if (res->error_code == ISOBUSFS_ERR_END_OF_FILE) {
ret = isobusfs_cli_send_and_register_fa_cf_event(priv,
ctx->handle,
cb, ctx);
Expand All @@ -750,21 +787,31 @@ isobusfs_cli_ls_handle_read_dir_sent(struct isobusfs_priv *priv,
}

ctx->state = ISOBUSFS_CLI_LS_STATE_CLOSE_DIR_SENT;
} else {
ctx->offset = ctx->entry_count;
ret = isobusfs_cli_send_and_register_fa_sf_event(priv,
ctx->handle, 0,
ctx->offset,
cb, ctx);
if (ret)
pr_int("Failed to send seek file request: %i\n", ret);
return;
}

ctx->state = ISOBUSFS_CLI_LS_STATE_SEEK_DIR_SENT;
if (!count) {
pr_int("Error: zero-length read without EOF\n");
goto error;
}

/*
* Directory seek offset is entry index, not byte offset.
* Server side seek rewinds and skips "offset" entries.
*/
ctx->offset = ctx->entry_count;

ret = isobusfs_cli_send_and_register_fa_sf_event(priv,
ctx->handle, 0,
ctx->offset,
cb, ctx);
if (ret)
pr_int("Failed to send seek file request: %i\n", ret);

ctx->state = ISOBUSFS_CLI_LS_STATE_SEEK_DIR_SENT;

return;
error:

ctx->state = ISOBUSFS_CLI_LS_STATE_ERROR;
}

Expand Down
62 changes: 40 additions & 22 deletions isobusfs/isobusfs_cli_selftests.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -603,23 +603,24 @@ struct isobusfs_cli_test_rf_path {
uint8_t flags;
uint32_t offset;
uint32_t read_size;
uint32_t expected_size;
bool expect_pass;
};

static struct isobusfs_cli_test_rf_path test_rf_patterns[] = {
/* expected result \\vol1\dir1\dir2\ */
{ "\\\\vol1\\dir1\\dir2\\file1k", 0, 0, 0, true },
{ "\\\\vol1\\dir1\\dir2\\file1k", 0, 0, 1, true },
{ "\\\\vol1\\dir1\\dir2\\file1k", 0, 1, 1, true },
{ "\\\\vol1\\dir1\\dir2\\file1k", 0, 2, 1, true },
{ "\\\\vol1\\dir1\\dir2\\file1k", 0, 3, 1, true },
{ "\\\\vol1\\dir1\\dir2\\file1m", 0, 0, 8, true },
{ "\\\\vol1\\dir1\\dir2\\file1m", 0, 0, 8 * 100, true },
{ "\\\\vol1\\dir1\\dir2\\file1m", 0, 100, 8 * 100, true },
{ "\\\\vol1\\dir1\\dir2\\file1m", 0, 0, ISOBUSFS_MAX_DATA_LENGH, true },
{ "\\\\vol1\\dir1\\dir2\\file1m", 0, 0, (ISOBUSFS_MAX_DATA_LENGH & ~3) + 16, true },
{ "\\\\vol1\\dir1\\dir2\\file1m", 0, 0, ISOBUSFS_MAX_DATA_LENGH + 1, true },
{ "\\\\vol1\\dir1\\dir2\\file1m", 0, 0, -1, true },
{ "\\\\vol1\\dir1\\dir2\\file1k", 0, 0, 0, 0, true },
{ "\\\\vol1\\dir1\\dir2\\file1k", 0, 0, 1, 1, true },
{ "\\\\vol1\\dir1\\dir2\\file1k", 0, 1, 1, 1, true },
{ "\\\\vol1\\dir1\\dir2\\file1k", 0, 2, 1, 1, true },
{ "\\\\vol1\\dir1\\dir2\\file1k", 0, 3, 1, 1, true },
{ "\\\\vol1\\dir1\\dir2\\file1m", 0, 0, 8, 8, true },
{ "\\\\vol1\\dir1\\dir2\\file1m", 0, 0, 8 * 100, 8 * 100, true },
{ "\\\\vol1\\dir1\\dir2\\file1m", 0, 100, 8 * 100, 8 * 100, true },
{ "\\\\vol1\\dir1\\dir2\\file1m", 0, 0, ISOBUSFS_MAX_DATA_LENGH, ISOBUSFS_MAX_DATA_LENGH, true },
{ "\\\\vol1\\dir1\\dir2\\file1m", 0, 0, (ISOBUSFS_MAX_DATA_LENGH & ~3) + 16, (ISOBUSFS_MAX_DATA_LENGH & ~3) + 16, true },
{ "\\\\vol1\\dir1\\dir2\\file1m", 0, 0, ISOBUSFS_MAX_DATA_LENGH + 1, ISOBUSFS_MAX_DATA_LENGH + 1, true },
{ "\\\\vol1\\dir1\\dir2\\file1m", 0, 0, UINT32_MAX, 1024 * 1024, true },
};

size_t current_rf_pattern_test;
Expand Down Expand Up @@ -678,7 +679,8 @@ static int isobusfs_cli_test_rf_req(struct isobusfs_priv *priv, bool *complete)
&test_rf_patterns[current_rf_pattern_test];
uint32_t actual_sum, expected_sum;
struct timespec current_time;
ssize_t remaining_size, read_size;
int64_t remaining_size;
Copy link
Copy Markdown
Contributor Author

@olerem olerem May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need here signed variable?

ssize_t read_size;
int ret;

clock_gettime(CLOCK_MONOTONIC, &current_time);
Expand Down Expand Up @@ -801,16 +803,23 @@ static int isobusfs_cli_test_rf_req(struct isobusfs_priv *priv, bool *complete)
free(priv->read_data);
priv->read_data = NULL;

remaining_size = (tp->offset + tp->read_size) -
(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) -
Copy link
Copy Markdown
Contributor Author

@olerem olerem May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to verify maximal sizes supported by protocol..

((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,
Copy link
Copy Markdown
Member

@marckleinebudde marckleinebudde May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

tp->read_size, tp->offset);
if (remaining_size < 0) {
pr_err("pattern test failed: %s. Read size is too big",
tp->path_name);
ret = -EINVAL;
goto test_fail;
} else if (remaining_size == 0) {
if (tp->read_size > tp->expected_size) {
pr_err("read test failed: %s. Server returned more data than expected file size (%u > %u)",
tp->path_name, tp->read_size, tp->expected_size);
ret = -EINVAL;
goto test_fail;
}
} else if (remaining_size > 0 && priv->read_data_len != 0) {
priv->read_offset += priv->read_data_len;

Expand All @@ -825,11 +834,20 @@ static int isobusfs_cli_test_rf_req(struct isobusfs_priv *priv, bool *complete)
goto test_fail;
test_start_time = current_time;
break;
} else if (remaining_size > 0 && priv->read_data_len == 0 && tp->expect_pass) {
pr_err("read test failed: %s. Read size is zero, but expected more data: %zd",
tp->path_name, remaining_size);
ret = -EINVAL;
goto test_fail;
} else if (remaining_size > 0 && priv->read_data_len == 0) {
if (tp->read_size > tp->expected_size &&
tp->read_size - remaining_size == tp->expected_size) {
/* this is acceptable case when read size
* is larger than actual file size
*/
pr_info("read test passed: %s. Reached end of file as expected.",
tp->path_name);
} else {
pr_err("read test failed: %s. Server returned zero bytes, but expected more data: %lld",
tp->path_name, (long long)remaining_size);
ret = -EINVAL;
goto test_fail;
}
}

/* fall troth */
Expand Down
Loading
Loading