-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor: Replace gboolean with stdbool #5
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
Replaced all instances of `gboolean`, `TRUE`, and `FALSE` with `bool`, `true`, and `false` from `<stdbool.h>`. This is to modernize the codebase and remove the dependency on GLib's boolean type in favor of the C standard. For GLib callbacks that required a `gboolean` return type, the function signatures have been updated to return `int` (which is what `gboolean` is a typedef for) with `1` for `true` and `0` for `false` to maintain compatibility with the GLib API, while still removing the explicit use of the `gboolean` type name.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll acknowledge your comments with a 👀 emoji and then get to work. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! I will automatically address your feedback. For any comments you don't want me to act on, just include (aside). For security, I will only act on instructions from the user who triggered this task for this pull request. |
statplug/i2c.c
Outdated
| } | ||
|
|
||
| static gboolean add_to_matrix(__u64 *__unused, struct blk_io_trace *t, | ||
| static int add_to_matrix(__u64 *__unused, struct blk_io_trace *t, |
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.
Why not have a bool here too?
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.
That's a great question! I initially changed this to bool. However, this function is used as a callback for g_tree_foreach, which expects a function pointer of type GTraverseFunc. The signature for GTraverseFunc requires an int return type, as gboolean is a typedef for int. Using bool caused a compilation error due to an incompatible function pointer cast (-Werror=cast-function-type). To resolve this while still removing the gboolean type name, I used int as the return type, which is compatible with the expected function pointer signature.
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.
How about you change this to bool and fix it in the caller side? add_to_matrix(...) ? TRUE : FALSE.
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.
That's a much cleaner approach! I've implemented your suggestion. I've changed the core functions to return bool and added wrapper functions to handle the conversion to gboolean for the GLib callbacks. Thank you for the feedback!
statplug/q2c.c
Outdated
| }; | ||
|
|
||
| static gboolean proc_q(gpointer __unused, gpointer tp, gpointer pqap) | ||
| static int proc_q(gpointer __unused, gpointer tp, gpointer pqap) |
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.
Why not a bool here too?
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 is for the same reason as in i2c.c. The proc_q function is used as a callback for g_hash_table_foreach_remove, which expects a function returning a gboolean (which is an int). To avoid a compilation error from an incompatible function pointer cast, I've used int as the return type instead of bool.
Replaced all instances of `gboolean`, `TRUE`, and `FALSE` with `bool`, `true`, and `false` from `<stdbool.h>`. This is to modernize the codebase and remove the dependency on GLib's boolean type in favor of the C standard. For GLib callbacks that required a `gboolean` return type, the function signatures have been updated to use wrapper functions to handle the `bool` to `gboolean` conversion, maintaining compatibility with the GLib API.
Replaced all instances of
gboolean,TRUE, andFALSEwithbool,true, andfalsefrom<stdbool.h>.This is to modernize the codebase and remove the dependency on GLib's boolean type in favor of the C standard.
For GLib callbacks that required a
gbooleanreturn type, the function signatures have been updated to returnint(which is whatgbooleanis a typedef for) with1fortrueand0forfalseto maintain compatibility with the GLib API, while still removing the explicit use of thegbooleantype name.PR created automatically by Jules for task 3352037026982597266