[riot-notifications] [RIOT-OS/RIOT] sys/ubjson: remove module. (#11724)
Juan I Carrano
notifications at github.com
Wed Jun 19 18:09:03 CEST 2019
**NOTE**: Marking as waiting because we need to deprecate it before removing.
The ubjson module has a number of quality defects and is unsafe. Considering CBOR is popular, standarized and supported in RIOT and that the ubjson implementation is a home-grown one whose API will likely be unfamiliar to new users, I propose to delete it.
This removal, of course, dows not have to be NOW. We can deprecate it for
one or two releases before.
What's wrong with this module?
- Unsafe: the parsing is done recursively. This is embedded in the API, so it is not possible to fix it without changing the API. A document with too much nesting can cause a stack overflow.
- Does not validate writing: it is possible to produce invalid output. From the docs:
> The library won't complain if you write multiple values that are not
> inside an array or object. The result will just not be properly serialized.
- Poorly tested. As shown by #11702, #11703 the tests were not even detecting that a False was stored as True.
- In line with the previous remark, see
Why is the following code in the unit tests??
- #2175 is still unfixed after 3.5 years.
- Code quality. The code has multiline macros that assign variables and return. See https://github.com/RIOT-OS/RIOT/blob/c3325148755c64a56b256253151a485498bcbc9b/sys/ubjson/ubjson-write.c#L34-L41
Can we mark it as deprecated this release and sweep it in the following one?
### Testing procedure
AFAIK there is nothing _in RIOT_ making use of this, so there is nothing to test. There might be third party code, so that's why we should first deprecate it.
### Issues/PRs references
See #2175, #11702, #11703
You can view, comment on, or merge this pull request online at:
-- Commit Summary --
* sys/ubjson: remove module.
-- File Changes --
D sys/include/ubjson.h (581)
D sys/ubjson/Makefile (1)
D sys/ubjson/ubjson-internal.h (62)
D sys/ubjson/ubjson-read.c (392)
D sys/ubjson/ubjson-write.c (189)
D tests/unittests/tests-ubjson/Makefile (1)
D tests/unittests/tests-ubjson/Makefile.include (2)
D tests/unittests/tests-ubjson/test-ubjson-empty-array.c (127)
D tests/unittests/tests-ubjson/test-ubjson-empty-object.c (130)
D tests/unittests/tests-ubjson/tests-ubjson.c (116)
D tests/unittests/tests-ubjson/tests-ubjson.h (54)
-- Patch Links --
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the notifications