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_entry_find, nvs_entry_next don't return proper error codes for differentiating between not found and internal errors (IDFGH-6149) #7826

Closed
Emill opened this issue Nov 3, 2021 · 2 comments
Assignees
Labels
Resolution: Done Issue is done internally Status: Resolved Issue is done internally Type: Feature Request Feature request for IDF

Comments

@Emill
Copy link

Emill commented Nov 3, 2021

Is your feature request related to a problem? Please describe.

The nvs iteration API looks very different from the rest of the nvs API. After reading #129, I now understand that this API was added later, so probably that's the reason it looks so different. The new API doesn't operate on handles and doesn't return proper error codes for the various situations when not a valid iterator could be returned.

The issue with this new API is that NULL is returned regardless of the error. According to the documentation, an iterator will be returned if there is a matching entry, and NULL if no entry satisfying criteria was found. But this is kind of a lie. In case, e.g. a malloc operation failed due to out of memory, flash read failed etc., NULL will be returned as well. This makes it impossible to differentiate when there actually are no entries found, and when some internal operation failed for other reasons. In case of out of memory, I as an application developer would try to free some other application memory and then try the nvs operation again, but now this is not possible.

Describe the solution you'd like

A different API with proper error codes so I can know for sure if no entries exist or some other error occurred. Proposedly, the API should also use nvs_handle_t instead of taking namespace as a string to be consistent with the rest of the API. The user probably wants to have such an opened handle anyway to later read the values being iterating over.

@Emill Emill added the Type: Feature Request Feature request for IDF label Nov 3, 2021
@espressif-bot espressif-bot added the Status: Opened Issue is new label Nov 3, 2021
@github-actions github-actions bot changed the title nvs_entry_find, nvs_entry_next don't return proper error codes for differentiating between not found and internal errors nvs_entry_find, nvs_entry_next don't return proper error codes for differentiating between not found and internal errors (IDFGH-6149) Nov 3, 2021
@0xjakob
Copy link
Contributor

0xjakob commented Nov 4, 2021

@Emill It's a nice suggestion, thank you for that! The problem is that we can't easily change a public API which is used already. We may change this during transition to IDF v5.0 but I can't promise this for 100%. We'll keep you updated on this issue.

@espressif-bot espressif-bot added Status: In Progress Work is in progress and removed Status: Opened Issue is new labels May 12, 2022
@0xjakob
Copy link
Contributor

0xjakob commented May 17, 2022

@Emill We are about to change the NVS iterator API now.
The new NVS iterator programming patter would look like this:

nvs_iterator_t it = nullptr;
esp_err_t res = nvs_entry_find(<nvs_partition_name>, <namespace>, NVS_TYPE_ANY, &it);
while(res == ESP_OK) {
    nvs_entry_info_t info;
    nvs_entry_info(it, &info); // Can omit error check if parameters are guaranteed to be non-NULL
    printf("key '%s', type '%d' \n", info.key, info.type);
    res = nvs_entry_next(&it);
}

The changed functions would look like this:

esp_err_t nvs_entry_find(const char *part_name,
        const char *namespace_name,
        nvs_type_t type,
        nvs_iterator_t *output_iterator);

esp_err_t nvs_entry_next(nvs_iterator_t *it);

esp_err_t nvs_entry_info(const nvs_iterator_t iterator, nvs_entry_info_t *out_info);

Feel free to try the following patch:
0001-refactor-nvs-New-interface-for-iterator-functions.zip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Done Issue is done internally Status: Resolved Issue is done internally Type: Feature Request Feature request for IDF
Projects
None yet
Development

No branches or pull requests

3 participants