-
-
Notifications
You must be signed in to change notification settings - Fork 495
Refactor RzConfig #5820
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: dev
Are you sure you want to change the base?
Refactor RzConfig #5820
Conversation
Rot127
left a comment
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.
Only checked the main files and hexagon. Looks like a way better architecture now.
Will do a full review later (currently pretty busy).
General request: Please document the high level design somewhere while you still remember the details.
| char *name; ///< Variable name | ||
| char *desc; ///< Description of the variable | ||
| RzList /*<char *>*/ *options; ///< Holds the string options | ||
| ut32 flags; ///< define the type of the data via RzConfigNodeFlags |
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.
| ut32 flags; ///< define the type of the data via RzConfigNodeFlags | |
| ut32 /*RzConfigNodeFlags*/ flags; ///< define the type of the data. |
|
|
||
| struct rz_config_hold_t { | ||
| RzConfig *cfg; | ||
| RzVector /* ConfigValue */ variables; |
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.
| RzVector /* ConfigValue */ variables; | |
| RzVector /*<ConfigValue>*/ variables; |
| bool lock; ///< When true forbids the add/remove of variables | ||
| void *user; ///< Pointer passed to the callbacks (getter, setter, setopt) | ||
| RzList /*<RzConfigNode *>*/ *nodes; ///< List of stored variables. | ||
| HtSP *ht; ///< HashMap for fast access to the nodes. |
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.
| HtSP *ht; ///< HashMap for fast access to the nodes. | |
| HtSP *node_index; ///< HashMap for fast access to the nodes. |
Why not use only a hash map?
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.
i have remove this (and i'm trying to figure it out where to place this) but essentially is to store the nodes in a sorted way, so when you iterate over them you always have the variables sorted.
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.
Wouldn't rz_list_add_sorted work?
I mean, does it need to be fast? The configs are no hot path.
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.
added sorted method.
| } | ||
|
|
||
| /** | ||
| * \brief Sets the value (type string list) of a given variable of the config |
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.
| * \brief Sets the value (type string list) of a given variable of the config | |
| * \brief Sets the value (type string list) of a given variable of the config. |
…alysis.vinfun analysis.vinfunrange
Your checklist for this pull request
RZ_APIfunction and struct this PR changes.RZ_API).Detailed description
Cleans up RzConfig and implements proper bindings.