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

NVS: Inconsistent behavior of nvs_read #86981

Open
sorru94 opened this issue Mar 12, 2025 · 2 comments
Open

NVS: Inconsistent behavior of nvs_read #86981

sorru94 opened this issue Mar 12, 2025 · 2 comments
Assignees
Labels
area: Storage Storage subsystem bug The issue is a bug, or the PR is fixing a bug

Comments

@sorru94
Copy link
Contributor

sorru94 commented Mar 12, 2025

The behavior of the NVS function nvs_read is not well defined when the arguments data and len are respectively set to NULL and 0 and when a corresponding NVS entry already exists.

The behavior differs on different boards. For example:

  • On the stm32h573i_dk, the function returns a value >0 indicating the size of the existing entry
  • On the mimxrt1064_evk, the function returns the value -22 indicating an incorrect argument

This issue has emerged with Zephyr v4.1.0.
Using Zephyr v3.7.0, both platforms would return a value >0 indicating the size of the existing entry.

To Reproduce

Replace the content of the standard NVS sample samples/subsys/nvs with the following snippet:

#include <zephyr/kernel.h>
#include <zephyr/sys/reboot.h>
#include <zephyr/device.h>
#include <string.h>
#include <zephyr/drivers/flash.h>
#include <zephyr/storage/flash_map.h>
#include <zephyr/fs/nvs.h>

#define NVS_PARTITION			storage_partition
#define NVS_PARTITION_DEVICE	FIXED_PARTITION_DEVICE(NVS_PARTITION)
#define NVS_PARTITION_OFFSET	FIXED_PARTITION_OFFSET(NVS_PARTITION)
#define NVS_PARTITION_SIZE		FIXED_PARTITION_SIZE(NVS_PARTITION)

int main(void)
{
	int flash_rc = flash_erase(NVS_PARTITION_DEVICE, NVS_PARTITION_OFFSET, NVS_PARTITION_SIZE);
	if (flash_rc) {
		printk("Flash erase failure: %s (%d).\n", strerror(-flash_rc), flash_rc);
		return -1;
	}

	const struct device *flash_device = NVS_PARTITION_DEVICE;
	if (!device_is_ready(flash_device)) {
		printk("Flash device %s not ready.\n", flash_device->name);
		return -1;
	}

	struct flash_pages_info fp_info = { 0 };
	off_t flash_offset = NVS_PARTITION_OFFSET;
	flash_rc = flash_get_page_info_by_offs(flash_device, flash_offset, &fp_info);
	if (flash_rc) {
		printk("Unable to get page info: %d.\n", flash_rc);
		return -1;
	}

	struct nvs_fs nvs_fs = { 0 };
	nvs_fs.flash_device = flash_device;
	nvs_fs.offset = flash_offset;
	nvs_fs.sector_size = fp_info.size;
	nvs_fs.sector_count = NVS_PARTITION_SIZE / fp_info.size;
	ssize_t nvs_rc = nvs_mount(&nvs_fs);
	if (nvs_rc) {
		printk("Mounting NVS failed: %d.\n", nvs_rc);
		return -1;
	}

	const char data[] = "some data";
	nvs_rc = nvs_write(&nvs_fs, 0U, data, sizeof(data));
	if (nvs_rc < 0) {
		printk("NVS write error: %s (%d).\n", strerror(-nvs_rc), nvs_rc);
		return -1;
	}

	// int foo = 0;
	// nvs_rc = nvs_read(&nvs_fs, 0U, &foo, sizeof(foo)); // This would work
	nvs_rc = nvs_read(&nvs_fs, 0U, NULL, 0); // This returns -22 on some boards
	if ((nvs_rc < 0) && (nvs_rc != -ENOENT)) {
		printk("NVS read error: %s (%d).\n", strerror(-nvs_rc), nvs_rc);
		return -1;
	}
	printk("NVS read returned: %s (%d).\n", strerror(-nvs_rc), nvs_rc);

	printk("Sample completed with no issue.\n");
	return 0;
}

Then build and flash on both boards with the standard example command:

west build -p always -b mimxrt1064_evk ./samples/subsys/nvs/
west build -p always -b stm32h573i_dk ./samples/subsys/nvs/

Expected behavior

Unclear, the documentation does not specify what the expected behavior for the NVS function should be.

Impact

This change in behavior between different zephyr versions could break existing code.

Environment (please complete the following information):

  • OS: Linux
  • Toolchain: Zephyr SDK 0.17.0
  • Tag: v4.1.0
@sorru94 sorru94 added the bug The issue is a bug, or the PR is fixing a bug label Mar 12, 2025
sorru94 added a commit to sorru94/astarte-device-sdk-zephyr that referenced this issue Mar 12, 2025
@henrikbrixandersen henrikbrixandersen added the area: Storage Storage subsystem label Mar 12, 2025
@Laczen
Copy link
Collaborator

Laczen commented Mar 12, 2025

@sorru94 that you are seeing different results between boards is a indication that there is a difference in the flash driver. As it should always be possible to do a read of length 0 in a NULL pointer (which should mean "do nothing"). I am suspecting that the mimixrt1064_evk flash driver does a bad check and returns an error.

@Laczen
Copy link
Collaborator

Laczen commented Mar 13, 2025

@sorru94 I have created a bug report for flash_mcux_flexspi_nor, when corrected the bug should disappear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Storage Storage subsystem bug The issue is a bug, or the PR is fixing a bug
Projects
None yet
Development

No branches or pull requests

3 participants