-
Notifications
You must be signed in to change notification settings - Fork 827
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
Name pointers should be constant #61
Comments
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. 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! |
Thankfully we have now a PR to address this long standing issue. Refer to #414 |
On every call that has a name in the parameters e.g.
tx_byte_pool_create
,tx_timer_create
, etc thename_ptr
should beconst CHAR *name_ptr
instead of the currentCHAR *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.
The text was updated successfully, but these errors were encountered: