Hash Table based RzGraph - #6152
Merged
Merged
Conversation
Rot127
requested review from
b1llow,
kazarmy,
notxvilka,
ret2libc,
thestr4ng3r and
wargio
as code owners
April 4, 2026 12:46
github-actions
Bot
added
rz-test
RzAnalysis
API
RzCore
RzUtil
RZIL
RzAsm-Assembler
labels
notxvilka
requested changes
Apr 4, 2026
notxvilka
left a comment
notxvilka
left a comment
Contributor
There was a problem hiding this comment.
Please fix the failing CI jobs (except old Debians - this seems unrelated to these changes)
Member
Author
|
Yes, I still work on moving the private structures to a private header and add getters for the struct members. |
Rot127
commented
Apr 4, 2026
Comment on lines
+1639
to
+1659
| /** | ||
| * \brief Returns the index of the node in the internal vector. | ||
| * | ||
| * DO NOT USE! | ||
| * | ||
| * It only exists for compatibility reasons and because refactoring is | ||
| * more effort than currently acceptable. | ||
| * | ||
| * Justification: | ||
| * The old implementations had exactly two identifiers for nodes. | ||
| * The pointer to the node itself, or its index into the internal list storing it. | ||
| * I skip the part how what a not good design decision this was. | ||
| * But in the new graph this data is considered private (index into lists/hash table) | ||
| * or are not expected by the graph user to be tracked (the individual node pointers). | ||
| * | ||
| * Sadly the json output of the graph still uses the vector indices as "ids". | ||
| * We can't replace them with the actual hash_id of a node. Because most graphs don't implement a hash function. | ||
| * For these cases the pointer is used internally as node hash. | ||
| * And printing it in json output would not be stable. | ||
| * Hence this hack. | ||
| */ |
Member
Author
There was a problem hiding this comment.
@notxvilka Resolve here? Or open an issue about it and fix later?
Contributor
There was a problem hiding this comment.
@Rot127 depends on the effort needed. If it's something that could be done quickly - here, if not - in a separate PRs.
Member
Author
There was a problem hiding this comment.
Let's better do it in a separated PR.
The fix requires all the graphs we have (see rz_core_graph()) to define a hash function.
This affects some tests and is hence easier to review in isolation.
This comment was marked as resolved.
This comment was marked as resolved.
Contributor
Good idea, but it requires the change in |
Member
Author
|
Also useful for |
Add comments Basic support for list and matrix based graph refactor Add rz_graph_*_new node and edge API Modify calling for rz graph edge data Add dfs and visitor mode Add new get nth neighbours Add unit test and wrapper for get edges Bug fixed and unit test Solve TODO about better semantic of get nodes Doc RZ_API and other functions, split impl to new files [cannot build] remove old graph impl and rename new graph API
Update Wrapper for graph identifier Ordered Nodes in graph drawable Fix icfg and several agraph refactor issues Use kahn to get topo sort in assign_layers Introduce khan for assign_layer and find DAG cycle and backedges in algorithm
Rot127
force-pushed
the
dist-rz_graph_refactor
branch
from
April 7, 2026 11:02
04febc0 to
9e8d29e
Compare
Rot127
force-pushed
the
dist-rz_graph_refactor
branch
from
April 7, 2026 11:36
9e8d29e to
6a64b2a
Compare
notxvilka
approved these changes
Apr 7, 2026
Codecov Report❌ Patch coverage is Additional details and impacted files
... and 12 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
4 tasks
Farhan-25
pushed a commit
to Farhan-25/rizin
that referenced
this pull request
Jun 19, 2026
* Basic implement of list based and matrix based graph Add comments Basic support for list and matrix based graph refactor Add rz_graph_*_new node and edge API Modify calling for rz graph edge data Add dfs and visitor mode Add new get nth neighbours Add unit test and wrapper for get edges Bug fixed and unit test Solve TODO about better semantic of get nodes Doc RZ_API and other functions, split impl to new files [cannot build] remove old graph impl and rename new graph API * Rewrite and replace agrach, cgraph, drawable_graph, il_graph with new API * Refactor graph API and fix unit test and multiple leaks Update Wrapper for graph identifier Ordered Nodes in graph drawable Fix icfg and several agraph refactor issues Use kahn to get topo sort in assign_layers Introduce khan for assign_layer and find DAG cycle and backedges in algorithm * Fix self loop and update db test * Solve request changes * Fix cbpf db * Fix test graph warning in testing NULL deletion * Fix mismatched rz_agraph_compute_layout nonnull * Fix memleak * Clean and fix code * Fix memleak in graph free * Fix memleak in matrix implement * Add bindgen doc * Fix memleak in early exit * Fix rz-bindgen warning comments * Make graph structures private. * Fix Windows C2036 * Fix typo in type annotation. * Add new type annotations for RzGraph and HtPP --------- Co-authored-by: Heersin <teablearcher@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Your checklist for this pull request
RZ_APIfunction and struct this PR changes.RZ_API).Detailed description
rz_core_graph().RZ_DEPRECATEDfunctions can be removed.Test plan
...
Closing issues
...