Skip to content

Commit

Permalink
sys/ubjson: remove module.
Browse files Browse the repository at this point in the history
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
  https://github.com/RIOT-OS/RIOT/blob/68dc5b0d6e0422df9c3653c2cb8021fc35974aae/tests/unittests/tests-ubjson/tests-ubjson.c#L66-L77
  Why is the following code in the unit tests??
  ```c
    irq_disable();
    sched_set_status(data->main_thread, STATUS_PENDING);
  ```
- #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?
  • Loading branch information
jcarrano authored and leandrolanzieri committed Dec 1, 2019
1 parent df7c424 commit bea30c3
Show file tree
Hide file tree
Showing 11 changed files with 0 additions and 1,670 deletions.
594 changes: 0 additions & 594 deletions sys/include/ubjson.h

This file was deleted.

1 change: 0 additions & 1 deletion sys/ubjson/Makefile

This file was deleted.

62 changes: 0 additions & 62 deletions sys/ubjson/ubjson-internal.h

This file was deleted.

Loading

0 comments on commit bea30c3

Please sign in to comment.