[riscv] Fix ISA feature parsing error from /proc/cpuinfo [v8/v8 : main]
Zhijin Zeng (Gerrit)
Zhijin Zeng added 1 comment![]( "Open in Gerrit")
This change only be active for risc-v 64 target.
Related details
- Anton Bikineev
- Florian Loitsch
- Ji Qiu
- Kasper Lund
- Yahan Lu (LuYahan)
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ji Qiu (Gerrit)
Ji Qiu added 1 comment![]( "Open in Gerrit")
How about merge the two HasListItem into one to minimize redundancy ?
Related details
- Anton Bikineev
- Florian Loitsch
- Kasper Lund
- Yahan Lu (LuYahan)
- Zhijin Zeng
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Zhijin Zeng (Gerrit)
Zhijin Zeng added 1 comment![]( "Open in Gerrit")
How about merge the two HasListItem into one to minimize redundancy ?
I also considered merging the two functions into one, but since isspace() checks for more than just the space character—it includes other whitespace characters like tabs, newlines, etc. Combining them would make the function less readable. I think it's better to keep them separate.
Related details
- Anton Bikineev
- Florian Loitsch
- Ji Qiu
- Kasper Lund
- Yahan Lu (LuYahan)
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Anton Bikineev (Gerrit)
Anton Bikineev added 1 comment![]( "Open in Gerrit")
Zhijin ZengHow about merge the two HasListItem into one to minimize redundancy ?
I also considered merging the two functions into one, but since isspace() checks for more than just the space character—it includes other whitespace characters like tabs, newlines, etc. Combining them would make the function less readable. I think it's better to keep them separate.
- Florian Loitsch
- Ji Qiu
- Kasper Lund
- Yahan Lu (LuYahan)
- Zhijin Zeng
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Zhijin Zeng (Gerrit)
Zhijin Zeng added 1 comment![]( "Open in Gerrit")
Zhijin ZengHow about merge the two HasListItem into one to minimize redundancy ?
Anton BikineevI also considered merging the two functions into one, but since isspace() checks for more than just the space character—it includes other whitespace characters like tabs, newlines, etc. Combining them would make the function less readable. I think it's better to keep them separate.
You could add a predicate as a template argument.
- Anton Bikineev
- Florian Loitsch
- Ji Qiu
- Kasper Lund
- Yahan Lu (LuYahan)
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Zhijin Zeng (Gerrit)
Zhijin Zeng added 1 comment![]( "Open in Gerrit")
Related details
- Anton Bikineev
- Florian Loitsch
- Ji Qiu
- Kasper Lund
- Yahan Lu (LuYahan)
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yahan Lu (LuYahan) (Gerrit)
Yahan Lu (LuYahan) added 1 comment![]( "Open in Gerrit")
Hi Clemens
Could you have time to review this CL?
Related details
- Anton Bikineev
- Clemens Backes
- Florian Loitsch
- Ji Qiu
- Kasper Lund
- Zhijin Zeng
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Clemens Backes (Gerrit)
Clemens Backes added 2 comments![]( "Open in Gerrit")
return HasListItem(list, item, [](const char c) { return isspace(c); });You can replace this lambda by just the "isspace" function, which will be passed as a function pointer.
[separator](const char c) { return c == separator; });Optional: If you want to go fancy, this lambda can be replaced by `std::bind_front(std::equal_to{}, separator)`.
And since this is only called twice (right after each other), I would consider making this a local helper, defined right before the two uses. Something like:
auto HasFeature = [features](const char* feature) {
return HasListItem(features, feature, std::bind_front(std::equal_to{}, '_'));
};Related details
- Anton Bikineev
- Florian Loitsch
- Ji Qiu
- Kasper Lund
- Zhijin Zeng
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Zhijin Zeng (Gerrit)
Zhijin Zeng added 3 comments![]( "Open in Gerrit")
Hi Clemens
Could you have time to review this CL?
Done
return HasListItem(list, item, [](const char c) { return isspace(c); });You can replace this lambda by just the "isspace" function, which will be passed as a function pointer.
Done
[separator](const char c) { return c == separator; });Optional: If you want to go fancy, this lambda can be replaced by `std::bind_front(std::equal_to{}, separator)`.
And since this is only called twice (right after each other), I would consider making this a local helper, defined right before the two uses. Something like:
auto HasFeature = [features](const char* feature) {
return HasListItem(features, feature, std::bind_front(std::equal_to{}, '_'));
};
- Anton Bikineev
- Clemens Backes
- Florian Loitsch
- Ji Qiu
- Kasper Lund
- Yahan Lu (LuYahan)
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |