Add tests for invalid file id = 0

[anglov]

Change behavior of meterfs, to start file_ids from 1

See: phoenix-rtos/phoenix-rtos-filesystems#152

Description

Motivation and Context

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Chore (refactoring, style fixes, git/CI config, submodule management, no code logic changes)

How Has This Been Tested?

• Already covered by automatic testing.
• New test added: self
• Tested by hand on: host-generic-pc

Checklist:

• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing linter checks and tests passed.
• My changes generate no new compilation warnings for any of the targets.

Special treatment

• This PR needs additional PRs to work (list the PRs, preferably in merge-order).
  Fix and improve meterfs tests #409
  meterfs: Start file id's from 1 phoenix-rtos-filesystems#152
• I will merge this PR by myself when appropriate.

[gemini-code-assist]

Summary of Changes

Hello @anglov, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the robustness of the meterfs file system by explicitly defining 0 as an invalid file identifier. It introduces changes to ensure that file lookup functions correctly propagate errors and that all subsequent operations attempted with a file ID of 0 consistently result in an ENOENT error, accompanied by new dedicated tests to validate this behavior.

Highlights

• Invalid File ID Handling: Established 0 as an invalid file ID within the meterfs system, ensuring that it is not used for valid file identifiers and that operations on it are correctly rejected.
• Error Propagation in Lookups: Modified file_lookup implementations in meterfs/file_pc.c and meterfs/file_phx.c to explicitly check for and propagate negative error codes returned by underlying lookup functions (hostflashsrv_lookup and lookup_rel). This ensures that file_lookup returns an error if the lookup fails, rather than potentially returning 0 or an incorrect ID.
• Comprehensive Test Coverage: Added a new test case, id_invalid, to meterfs/test_meterfs_miscellaneous.c. This test rigorously verifies that all meterfs operations (e.g., file_getInfo, file_close, file_write, file_read, file_resize) performed with fileId = 0 consistently return -ENOENT, confirming the intended behavior for invalid file IDs.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

The code changes modify file_lookup functions to return file IDs instead of status codes and introduce a new test case to validate the behavior for file ID 0. There are potential integer overflows in the modified functions and opportunities to improve the robustness and consistency of the new tests.

[github-actions]

Unit Test Results

9 446 tests  +9   8 857 ✅ +11   49m 0s ⏱️ -43s
  545 suites  - 1     589 💤 ± 0 
    1 files   ±0       0 ❌  -  2 

Results for commit 2948529. ± Comparison against base commit 0b83bc9.

This pull request removes 2 and adds 11 tests. Note that renamed tests count towards both.

phoenix-rtos-tests/libc/pthread ‑ aarch64a53-zynqmp-qemu:phoenix-rtos-tests/libc/pthread
phoenix-rtos-tests/psh/touch-rootfs ‑ armv7m4-stm32l4x6-nucleo:phoenix-rtos-tests/psh/touch-rootfs

phoenix-rtos-tests/libc/pthread ‑ aarch64a53-zynqmp-qemu:phoenix-rtos-tests/libc/pthread.test_pthread_cleanup.pthread_cleanup_push_exit
phoenix-rtos-tests/libc/pthread ‑ aarch64a53-zynqmp-qemu:phoenix-rtos-tests/libc/pthread.test_pthread_cleanup.pthread_cleanup_push_pop_exec
phoenix-rtos-tests/libc/pthread ‑ aarch64a53-zynqmp-qemu:phoenix-rtos-tests/libc/pthread.test_pthread_cleanup.pthread_cleanup_push_pop_exec_pthread_exit
phoenix-rtos-tests/libc/pthread ‑ aarch64a53-zynqmp-qemu:phoenix-rtos-tests/libc/pthread.test_pthread_cleanup.pthread_cleanup_push_pop_no_exec
phoenix-rtos-tests/libc/pthread ‑ aarch64a53-zynqmp-qemu:phoenix-rtos-tests/libc/pthread.test_pthread_cond.pthread_cond_timedwait_fail_broadcast_incorrect_timeout
phoenix-rtos-tests/libc/pthread ‑ aarch64a53-zynqmp-qemu:phoenix-rtos-tests/libc/pthread.test_pthread_cond.pthread_cond_timedwait_fail_signal_incorrect_timeout
phoenix-rtos-tests/libc/pthread ‑ aarch64a53-zynqmp-qemu:phoenix-rtos-tests/libc/pthread.test_pthread_cond.pthread_cond_timedwait_pass_broadcast
phoenix-rtos-tests/libc/pthread ‑ aarch64a53-zynqmp-qemu:phoenix-rtos-tests/libc/pthread.test_pthread_cond.pthread_cond_timedwait_pass_signal
phoenix-rtos-tests/libc/pthread ‑ aarch64a53-zynqmp-qemu:phoenix-rtos-tests/libc/pthread.test_pthread_cond.pthread_cond_wait_broadcast
phoenix-rtos-tests/libc/pthread ‑ aarch64a53-zynqmp-qemu:phoenix-rtos-tests/libc/pthread.test_pthread_cond.pthread_cond_wait_signal
…

♻️ This comment has been updated with latest results.

[anglov]

/gemini review

[gemini-code-assist]

Code Review

This pull request adds a new test case, id_invalid, to verify that operations on file ID 0 are correctly handled as invalid, returning -ENOENT. This aligns with the change to start file IDs from 1. The new test covers several file operations. My review found a couple of potential issues in the new test case: a likely incorrect path passed to file_lookup and a misleading error message in an assertion. I've provided suggestions to fix these.

[anglov]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a new test case, id_invalid, to ensure that file operations with an invalid file ID of 0 are correctly handled by returning -ENOENT. The test is well-structured, covering a good range of file operations and correctly setting up the test conditions. The changes are clear and effectively validate the intended behavior. I have one suggestion to improve the readability of an assertion within the new test.

[damianloew]

task id missing, above that lgtm

[anglov]

It's not connected with any existing task.

[damianloew]

It's not connected with any existing task.

Ok, I see that the change in filesystems also didn't have the associated task

[damianloew]

Remember about marking those checkboxes befoe merge:
All new and existing linter checks and tests passed.
My changes generate no new compilation warnings for any of the targets.