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

Need to sort out storage (PersistentStorageDelegate and KVS) behavior for 0-length nullptr data #16130

Closed
bzbarsky-apple opened this issue Mar 11, 2022 · 5 comments
Labels
stale Stale issue or PR

Comments

@bzbarsky-apple
Copy link
Contributor

Problem

Some of our storage backends allow storing a nullptr with 0 length; others do not. We should be consistent and either always allow it (preferred) or always disallow it. The current setup means that code can work against one backend but fail against another, reducing the value of testing we do.

Proposed Solution

  1. Document in PersistentStorageDelegate what the expected behavior is.
  2. Document in KeyValueStoreManager what the expected behavior is.
  3. Align implenentations with the documentation.
tcarmelveilleux added a commit to tcarmelveilleux/connectedhomeip that referenced this issue Mar 11, 2022

Verified

This commit was signed with the committer’s verified signature.
mtreinish Matthew Treinish
- Make Server's PersistentStorageDelegate and
  TestPersistentStorage delegate properly handle nullptr
  input for zero-length.
- Document behavior expected of PersistentStorageDelegate interface

Issue project-chip#16130
andy31415 pushed a commit that referenced this issue Mar 14, 2022
* Fix PersistentStorageDelegate 0-storage behavior

- Make Server's PersistentStorageDelegate and
  TestPersistentStorage delegate properly handle nullptr
  input for zero-length.
- Document behavior expected of PersistentStorageDelegate interface

Issue #16130

* Add unit tests for TestPersistentStorageDelegate

* Address review comments

* Restyled by clang-format

* Apply suggestions from code review

Co-authored-by: Boris Zbarsky <[email protected]>

Co-authored-by: Restyled.io <[email protected]>
Co-authored-by: Boris Zbarsky <[email protected]>
andrei-menzopol pushed a commit to andrei-menzopol/connectedhomeip that referenced this issue Apr 14, 2022
* Fix PersistentStorageDelegate 0-storage behavior

- Make Server's PersistentStorageDelegate and
  TestPersistentStorage delegate properly handle nullptr
  input for zero-length.
- Document behavior expected of PersistentStorageDelegate interface

Issue project-chip#16130

* Add unit tests for TestPersistentStorageDelegate

* Address review comments

* Restyled by clang-format

* Apply suggestions from code review

Co-authored-by: Boris Zbarsky <[email protected]>

Co-authored-by: Restyled.io <[email protected]>
Co-authored-by: Boris Zbarsky <[email protected]>
@stale
Copy link

stale bot commented Sep 8, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale Stale issue or PR label Sep 8, 2022
@bzbarsky-apple bzbarsky-apple removed the stale Stale issue or PR label Sep 9, 2022
@stale
Copy link

stale bot commented Mar 11, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale Stale issue or PR label Mar 11, 2023
@bzbarsky-apple bzbarsky-apple removed the stale Stale issue or PR label Mar 13, 2023
@bzbarsky-apple
Copy link
Contributor Author

It's possible that the storage audit addressed this....

@stale
Copy link

stale bot commented Oct 15, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale Stale issue or PR label Oct 15, 2023
@bzbarsky-apple
Copy link
Contributor Author

Fixed by storage audit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Stale issue or PR
Projects
None yet
Development

No branches or pull requests

1 participant