threadx
threadx copied to clipboard
Name pointers should be constant
On every call that has a name in the parameters e.g. tx_byte_pool_create
, tx_timer_create
, etc the name_ptr
should be const CHAR *name_ptr
instead of the current CHAR *name_ptr
.
It's not only a GP to give that hint to the compiler but also makes the code usable in C++ without having to add a cast on each of those names. (ISO C++ forbids converting a string constant to 'CHAR*').
If you want it, I have a PR ready with this fix.
Very good point. I support this as well. Having those C strings non const is a major nuisance, in particular for C++ users. Our code is cluttered with unnecessary casts and #pragma GCC diagnostic ignored "-Wwrite-strings" statements to make the code compile with C++.
Hi All,
We on the RTOS team agree with you - we'd like to make this const. Unfortunately, we can not change the existing API. This change requires lots of documentation work (API definitions/example code/etc.), coding, and testing. This also goes well beyond just ThreadX – all components and add-on protocols would need the same. ThreadX and most of our middleware is going through the re-certification process and this change can't be made right now.
(const) Sorry.
hey @goldscott
Thanks for the quick response. And I understand the certification part and that you can't make this change right now.
As for the rest, let me stress that this only involves updating the documentation on the API declaration (apart from the code, of course). No changes are required in the samples, example code and explanations. In the end this acts a compiler hint and letting the caller know that the callee wont change the content of the string.
Plus this is not a breaking change in the sense that the existing code has to be modified to use the new declaration.
Last thought: I understand that developers want stability (and I count myself among those) but that should never prevent improving things and moving forward. Having said that one can always provide alternative APIs and mark others as obsolete and plan for their removal in X versions.
Please give it another tough or, at least, put it on the backlog for the spring cleaning.
@josesimoes we agree with you that we will review this in the future. Close the issue now. Feel free to reopen it or reach out to us if you have additional thoughts or comments. Thanks!
any updates on when this issue will be worked on?
as @josesimoes said, this fix should be backwards compatible with all existing code and examples. It shouldn't break the API for existing users
Hi @csrichter @josesimoes @hwmaier - we are working on this now!