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

[th/c-list] import c-list library as third-party module for internal use #412

Merged
merged 3 commits into from
Nov 18, 2024

Conversation

thom311
Copy link
Owner

@thom311 thom311 commented Nov 14, 2024

We already have an internal implementation for a circular, intrusive,
doubly-linked list (in include/netlink/list.h).

However, I think the "c-list" library is excellent, and in 2024 a
netlink library should no longer reimplement a basic data structure.

Vendor "c-list" via git-subtree. We want to strictly follow upstream (no
local changes on our side) but re-import the library when there are new
upstream changes.

Existing users of <netlink/list.h> are not required to be rewritten. But
new users should use "c-list.h". This is only an internal dependency, we
anyway wouldn't want to expose such a list in our public API.

[1] https://github.com/c-util/c-list

Imported via:

$ git subtree add --prefix third_party/c-list/ [email protected]:c-util/c-list.git main --squash

Update with:

$ git subtree pull --prefix third_party/c-list/ [email protected]:c-util/c-list.git main --squash

git-subtree-dir: third_party/c-list
git-subtree-split: 9aa81d84cadc67e92b441c89f84c57e72dd1e8a9
…hird_party/c-list'

Add "c-list" library ([1]).

We already have an internal implementation for a circular, intrusive,
doubly-linked list (in `include/netlink/list.h`).

However, I think the "c-list" library is excellent, and in 2024 a
netlink library should no longer reimplement a basic data structure.

Vendor "c-list" via git-subtree. We want to strictly follow upstream (no
local changes on our side) but re-import the library when there are new
upstream changes.

Existing users of <netlink/list.h> are not required to be rewritten. But
new users should use "c-list.h". This is only an internal dependency, we
anyway wouldn't want to expose such a list in our public API.

[1] https://github.com/c-util/c-list

Imported via:

  $ git subtree add --prefix third_party/c-list/ [email protected]:c-util/c-list.git main --squash

Update with:

  $ git subtree pull --prefix third_party/c-list/ [email protected]:c-util/c-list.git main --squash
@ronand-atl
Copy link
Contributor

LGTM 👍

Could it be good to mention in the existing list implementation that new modules should prefer to use this implementation instead?

@thom311
Copy link
Owner Author

thom311 commented Nov 18, 2024

Could it be good to mention in the existing list implementation that new modules should prefer to use this implementation instead?

This is mentioned both in the commit message and in the PR description here. Do you have anything else in mind?

@ronand-atl
Copy link
Contributor

Probably just a note at the top of the old list.h file. Sorry I should have specified that.

@thom311 thom311 merged commit d4d1eb7 into main Nov 18, 2024
8 checks passed
@thom311 thom311 deleted the th/c-list branch November 18, 2024 21:58
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.

2 participants