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

[DRAFT] API: implement APIs from the new specification #296

Closed
wants to merge 6 commits into from

Conversation

daniloegea
Copy link
Collaborator

  • Implement some of the new APIs described in [1] and add some functions that were intended for internal use to the public header files.
  • Implement/change unit tests related to the changes.
  • Introduce some unit tests written in C using the cmocka framework. The idea is to make it easier to catch runtime problems like memory errors. One can just build the tests with -fsanitize=address or run them with valgrind to check for problems. The new files will not be compiled by default, the new option -Dunit_testing=true must be used during the meson setup step.
  • Integrate the new tests with the meson build configuration.
  • Add a .gcovr file to exclude the new tests from the coverage test. It a future commit we might just move the new tests directory to another place.

[1] - https://discourse.ubuntu.com/t/spec-api-and-bindings-definition-for-libnetplan/29106

Description

It's a work in progress, some APIs were not implemented yet.

Checklist

  • Runs make check successfully.
  • Retains 100% code coverage (make check-coverage).
  • New/changed keys in YAML format are documented.
  • (Optional) Adds example YAML for new feature.
  • (Optional) Closes an open bug in Launchpad.

 * Implement some of the new APIs described in [1] and add some
   functions that were intended for internal use to the public header
   files.
 * Implement/change unit tests related to the changes.
 * Introduce some unit tests written in C using the cmocka framework.
   The idea is to make it easier to catch runtime problems like memory
   errors. One can just build the tests with -fsanitize=address or run
   them with valgrind to check for problems. The new files will not be
   compiled by default, the new option -Dunit_testing=true must be used
   during the meson setup step.
 * Integrate the new tests with the meson build configuration.
 * Add a .gcovr file to exclude the new tests from the coverage test. It
   a future commit we might just move the new tests directory to another
   place.

[1] - https://discourse.ubuntu.com/t/spec-api-and-bindings-definition-for-libnetplan/29106
@slyon slyon marked this pull request as draft November 16, 2022 12:56
Add back two symbols removed in previous commit as aliases to avoid
changes in the ABI.
    * generate.c: free the options context allocated at the start of the main
      function.
    * openvswitch.c: make sure the GString "cmds" is free'd before the
      function returns.
    * parse.c: free the temporary list "values" before the function
      returns.
    * types.c: in this particular case we g_strdup both key and value
      strings when inserting them in the hash table in
      parse.c:handle_generic_map(). Calling the destructor for both of
      them will make sure all the memory is free'd. When using
      free_hashtable_with_destructor in the future we'll need to make
      sure both key and value are not poiting to the same place to avoid
      a double free.
    * parse.c: parsed_defs is set to NULL preventing
      netplan_parser_reset() to free the memory allocated for the
      hash table. Not setting it to NULL is safe because all the data is moved to
      another hash table, leaving parsed_defs with no elements. Not
      nullifying parsed_defs here allows netplan_parser_reset() to free
      it.
Definitions used by other public header files were
moved to a new types.h file. The new file was included where it's needed
and it shouldn't break external consumers.

Also:
    * Add new netplan_error functions with tests
    * Enable cmocka unittests during coverage test
Add the new header file to the meson install list. Move the new members
in the struct netplan_net_definition to the end of the struct to avoid
breaking the ABI.
@daniloegea
Copy link
Collaborator Author

I will close this PR in favor of a clean one.

@daniloegea daniloegea closed this Nov 24, 2022
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

Successfully merging this pull request may close these issues.

1 participant