-
Couldn't load subscription status.
- Fork 881
const: Add const keyword for all name pointer parameters #414
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
|
@eclipse-threadx/iot-threadx-committers I need one of you to review this PR, please. |
|
Thank you for this contribution, @MaJerle. I asked the group of committers to review it. I appreciate your patience. Before we can merge this PR, you need to sign the Eclipse Contributor Agreement (ECA). The purpose of the ECA is to provide a written record that you have agreed to provide your code and documentation contributions under the licenses used by the Eclipse ThreadX project. It also makes it clear that you are promising that what you are contributing to Eclipse is code you wrote, and you have the necessary rights to contribute it to our projects. And finally, it documents a commitment from you that your open source contributions will be permanently on the public record. Signing the ECA requires an Eclipse Foundation account if you do not already have one. You can create one for free at https://accounts.eclipse.org. Be sure to use the same email address when you register for the account that you intend to use on Git commit records. |
|
In principle, I agree with this request, and we should have included this in the original APIs. However, now that the API has been public for 28 years, I would refrain from making this change. I don't like anything that alters the published API. That said, if we were to do this, we would also have to update the user guide and, I think, the name pointer storage in all the control blocks, e.g., TX_THREAD, TX_QUEUE, etc. |
I may sound nasty, but if it was not done correctly for |
|
I'm also interested by having const qualifier for name. I understand the point raised about API stability and certification. @billlamiework would it be ok to use a // tx_user.h
#define TX_OBJECT_NAME_CONST
// tx_api.h
#ifdef TX_OBJECT_NAME_CONST
typedef const CHAR* TX_NAME_PTR;
#else
typedef CHAR* TX_NAME_PTR;
#endif
typedef struct TX_THREAD_STRUCT
{
...
TX_NAME_PTR tx_thread_name; /* Pointer to thread's name */
...
} TX_THREAD;
UINT _tx_thread_info_get(TX_THREAD *thread_ptr, TX_NAME_PTR* name, UINT *state, ULONG *run_count,
UINT *priority, UINT *preemption_threshold, ULONG *time_slice,
TX_THREAD **next_thread, TX_THREAD **next_suspended_thread); |
This pull requests fixes the compiler warning when creating TX objects with literal strings since we are passing literal string to the function accepting
CHAR *, that can, by design, modify input parameter and therefore cause undefined system behavior.All name pointers become
const CHAR*type.There is no backward incompatibility when creating objects, but there is a potential backward in-compatibility when calling
tx_<modulename>_name_getfunction, because parameter shall be changed toconst char* variable.Currently we can get the name as:
New required function call:
I've not seen tests for getting the name in the threadx, I also clearly assume that people using
_getapi are the minority.From safety and quality standpoint, this upgrade makes a lot of sense.
PR checklist