-
Notifications
You must be signed in to change notification settings - Fork 316
Add CBOR Support to libyang (#2130) #2449
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
base: master
Are you sure you want to change the base?
Conversation
feat: Add initial CBOR format support with libcbor integration (squahsed commit) - Add ENABLE_CBOR_SUPPORT CMake flag to conditionally enable CBOR functionality - Integrate libcbor dependency with CMake find_package and conditional compilation - Add basic parser_cbor.c and parser_cbor.h files with foundational structures - Implement initial CBOR parsing functions with libcbor as low-level parser - Add necessary format switch statements throughout codebase for CBOR support - Introduce lyd_parse_data_mem_len() high-level function for parsing CBOR from memory (required because CBOR binary data may contain null bytes, making strlen() unreliable) - Build complete CBOR parser that constructs libyang data trees - Link parsed CBOR data with schema trees for validation - Fix initial memory allocation errors and remove dead code This commit establishes the foundation for CBOR support in libyang, allowing the library to be built with or without CBOR capabilities based on build configuration. ```
- arranged all the high level parser code in parser_cbor.c - added all the common high level ctx variables to parser_internal.h - added lcbor.c and lcbor.h as a wrapper over libcbor but to keep the coding style consistent
- made small changes to printer_cbor.c - and made small function renaming in parser_cbor
- Reorganize high-level parser code in parser_cbor.c - Add common high-level context variables to parser_internal.h - Create lcbor.c and lcbor.h as a consistent wrapper over libcbor - Maintain coding style consistency with existing libyang modules
- consistent function structure: all functions now mirror their JSON counterparts - lydcbor_parse_name() - parses member names with @ and : support - lydcbor_get_node_prefix() - handles module prefixes - lydcbor_get_snode() - finds schema nodes - lydcbor_value_type_hint() - determines value type hints - lydcbor_data_check_opaq() - checks if data should be parsed as opaque - lydcbor_metadata_finish() - links forward-referenced metadata - lydcbor_meta_attr() - parses metadata/attributes - lydcbor_parse_any() - handles anydata/anyxml - lydcbor_parse_instance_inner() - parses inner nodes - lydcbor_parse_instance() - parses single node instances - lydcbor_subtree_r() - main recursive parsing function - using cbor_typeof() consistently to check types instead of multiple cbor_isa_*() calls - better error handling, null handling and array processing and meta data support and qpaque node support - introduced LY_VALUE_CBOR format and added all necessary case statements
- Parallel structure to printer_json.c for consistency - Full support for all YANG node types (leaf, leaf-list, list, container, anydata, anyxml, RPC, action, notification)
|
Thanks a lot for this effort and I will try to provide some feedback today or tomorrow. But I can immediately tell you to change the libyang base branch to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally looks fine but some improvements are possible and the coding style is somewhat different to ours.
| option(ENABLE_TOOLS "Build binary tools 'yanglint' and 'yangre'" ON) | ||
| option(ENABLE_COMMON_TARGETS "Define common custom target names such as 'doc' or 'uninstall', may cause conflicts when using add_subdirectory() to build this project" ON) | ||
| option(BUILD_SHARED_LIBS "By default, shared libs are enabled. Turn off for a static build." ON) | ||
| option(ENABLE_CBOR_SUPPORT "Enable CBOR support with libcbor" ON) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, an option is probably redundant. You can just search for libcbor and if found, the support will be added, otherwise not.
| endif() | ||
|
|
||
| if(ENABLE_CBOR_SUPPORT) | ||
| find_package(PkgConfig) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is pkg-config really required? We have used it only when necessary, the else() branch should be fine except may be better to add a new FindCBOR.cmake module file.
| * https://opensource.org/licenses/BSD-3-Clause | ||
| */ | ||
|
|
||
| #ifdef ENABLE_CBOR_SUPPORT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems redundant if the define is around the whole file, the file should just not be compiled at all if disabled. In all the new files.
| @@ -0,0 +1,131 @@ | |||
| /** | |||
| * @file lcbor.h | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just cbor.c would probably fit better, like lyb.c and lyb.h.
| */ | ||
| void lycbor_ctx_free(struct lycbor_ctx *cborctx) | ||
| { | ||
| if (cborctx) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant if and maybe the whole function.
| struct lylyb_ctx *lybctx; /* LYB context */ | ||
| }; | ||
|
|
||
| #ifdef ENABLE_CBOR_SUPPORT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really needed, I think, if you just declare struct lycbor_ctx;.
| struct lyd_node **first_p, struct ly_in *in, uint32_t parse_opts, uint32_t val_opts, uint32_t int_opts, | ||
| struct ly_set *parsed, ly_bool *subtree_sibling, struct lyd_ctx **lydctx_p); | ||
|
|
||
| #ifdef ENABLE_CBOR_SUPPORT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again not necessary, the function can exist and fail. I would like to keep these macros to a minimum.
| } | ||
|
|
||
| /* unknown data node */ | ||
| printf("checkpoint1-json\n"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leftovers.
| /* but first process children of the object in the array */ | ||
| do { | ||
| LY_CHECK_GOTO(ret = lydjson_subtree_r(lydctx, *node_p, lyd_node_child_p(*node_p), NULL), cleanup); | ||
| LY_CHECK_GOTO(ret = lydjson_subtree_r(lydctx, *node_p, (*node_p), NULL), cleanup); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This changes how JSON opaque nodes are stored, what is the reason?
| escaped when the anydata is printed in XML format. */ | ||
| LYD_ANYDATA_XML, /**< Value is a string containing the serialized XML data. */ | ||
| LYD_ANYDATA_JSON, /**< Value is a string containing the data modeled by YANG and encoded as I-JSON. */ | ||
| LYD_ANYDATA_CBOR, /**< Value is a string containing the data modeled by YANG and encoded as CBOR. */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LYD_ANYDATA_LYB was removed in the devel branch and I think for the same reason LYD_ANYDATA_CBOR is not required. Only objects are allowed in anydata, so it should always be possible to parse them into LYD_ANYDATA_DATATREE.
Add CBOR Support to libyang
Attempt to resolve #2130
Overview
This PR adds comprehensive CBOR support to libyang, enabling parsing, validating, and printing instance data in CBOR format as specified in RFC 9254. The implementation uses libcbor as the underlying CBOR library and follows libyang's existing patterns and conventions.
Key Features
ENABLE_CBOR_SUPPORT, allowing builds with or without CBOR capabilitiesprinter_json.c, supporting all YANG node types (leaf, leaf-list, list, container, anydata, anyxml, RPC, action, notification)Implementation Details
New Files
parser_cbor.c/parser_cbor.h- High-level CBOR parserprinter_cbor.c/printer_cbor.h- CBOR printer implementationlcbor.c/lcbor.h- Consistent wrapper over libcbor following libyang coding styleChanges
ENABLE_CBOR_SUPPORTCMake flag to conditionally enable CBOR functionalityLY_VALUE_CBORformat with corresponding case statements throughout the codebaselyd_parse_data_mem_len()high-level function for parsing CBOR from memory (required because CBOR binary data may contain null bytes, makingstrlen()unreliable)TODO / Known Limitations
lyd_parse_data_mem_lento input handler (in.c) for better architectureDependencies
ENABLE_CBOR_SUPPORT=ONBuild Instructions
Testing :
Considered an
example.jsonfile :Converted the same into cbor notation and output in
example.cborusing standard cbor libraries in python (below code - json_to_cbor.py):Now ran the following json_test.c on the json file with the necessary yang modules
iana-if-type.yangandietf-interfaces.yangin the directory to see functionality on the json dataRepeated the same with the
cbor_test.cfile (in the same directory) utilizing the cbor features introduced to test the newly built libraryFollowing output:
And we can change the input file from
example.cbortooutput.cborto see the parsed data being parsed properly