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

Invalid len argument type in NVS C++ API? (IDFGH-4275) #6123

Closed
hg opened this issue Nov 17, 2020 · 2 comments
Closed

Invalid len argument type in NVS C++ API? (IDFGH-4275) #6123

hg opened this issue Nov 17, 2020 · 2 comments

Comments

@hg
Copy link

hg commented Nov 17, 2020

Environment

  • IDF version: v4.3-dev-1720-g494a124d9
  • Compiler version: xtensa-esp32-elf-gcc (crosstool-NG esp-2020r3) 8.4.0

Problem Description

Hi,

I omitted most of the template as it doesn't really apply to this issue.

I think the NVSHandle::get_string method uses the wrong type for argument len:

virtual esp_err_t get_string(const char *key, char* out_str, size_t len) = 0;

If we look at the docs, it is described as the pointer to the variable which receives the length of the string actually read from flash memory:

* @param[inout] length A non-zero pointer to the variable holding the length of out_value.
* It will be set to the actual length of the value
* written. For nvs_get_str this includes the zero terminator.

And if we look at the C API, the corresponding function does receive a pointer:

esp_err_t nvs_get_str (nvs_handle_t handle, const char* key, char* out_value, size_t* length);

It can be worked around by using NVSHandle::get_item_size and then passing its result to NVSHandle::get_string, but I guess docs for NVSHandle::get_string could be improved if it's too late to change its signature.

Thank you!

@github-actions github-actions bot changed the title Invalid len argument type in NVS C++ API? Invalid len argument type in NVS C++ API? (IDFGH-4275) Nov 17, 2020
@0xjakob
Copy link
Contributor

0xjakob commented Dec 1, 2020

@hg
Thanks a lot for being attentive!
We will change the documentation. This is in fact a documentation mistake from copy-pasting the docs over from the C API.

@0xjakob
Copy link
Contributor

0xjakob commented Feb 20, 2021

@hg A fix has been merged into our internal CI system. It should be pushed to the github master during the next few weeks.

projectgus pushed a commit that referenced this issue Jun 30, 2021
* Move nvs flash README to common doc directory
* correct markup of functions and types in text
  from old README
* Better comment of nvs_get_used_entry_count()
* Mention C++ example in API reference
* Used target instead of hard code ESP32
* Note that strings can only span one page
* Reflect that item types have been moved
* Some clarification about nvs_commit()
* Improved reference to the ESP Partition API
* fixed little mistake in documenting-code.rst
* Change of nvs_open_from_part() to
  nvs_open_from_partition() reflected in docs
* Corrected documentation of
  NVSHandle::get_string(), NVSHandle::get_blob()
  and NVSHandle::get_item_size().

* Closes DOC-165
* Closes IDF-1563
* Closes IDF-859
* Closes #6123
espressif-bot pushed a commit that referenced this issue Jul 18, 2021
* Better comment of nvs_get_used_entry_count()
* Mention C++ example in API reference
* WIP: Used target instead of hard code ESP32
* Note that strings can only span one page
* Reflect that item types have been moved
* Some clarification about nvs_commit()
* Improved reference to the ESP Partition API
* fixed little mistake in documenting-code.rst
* Change of nvs_open_from_part() to
  nvs_open_from_partition() reflected in docs
* Corrected documentation of
  NVSHandle::get_string(), NVSHandle::get_blob()
  and NVSHandle::get_item_size().

* Closes IDF-1563
* Closes IDF-859
* Closes #6123
espressif-bot pushed a commit that referenced this issue Dec 26, 2021
* Better comment of nvs_get_used_entry_count()
* Mention C++ example in API reference
* WIP: Used target instead of hard code ESP32
* Note that strings can only span one page
* Reflect that item types have been moved
* Some clarification about nvs_commit()
* Improved reference to the ESP Partition API
* fixed little mistake in documenting-code.rst
* Change of nvs_open_from_part() to
  nvs_open_from_partition() reflected in docs
* Corrected documentation of
  NVSHandle::get_string(), NVSHandle::get_blob()
  and NVSHandle::get_item_size().

* Closes IDF-1563
* Closes IDF-859
* Closes #6123
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants