Skip to content
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

Why ThreadX entry function use ULONG as entry input instead of a void *? #413

Open
parmi93 opened this issue Oct 1, 2024 · 25 comments
Open

Comments

@parmi93
Copy link

parmi93 commented Oct 1, 2024

UINT tx_thread_create(TX_THREAD *thread_ptr, CHAR *name_ptr, 
                      VOID (*entry_function)(ULONG), ULONG entry_input, 
                      VOID *stack_start, ULONG stack_size, UINT priority,
                      UINT preempt_threshold, ULONG time_slice, 
                      UINT auto_start)

Documentation

image

If I need to pass more than one parameter to the entry_function I must necessarily use a void *, hence my question, why ThreadX uses a ULONG instead of a void * like FreeRTOS does?

@MaJerle
Copy link

MaJerle commented Oct 1, 2024

It could be because of certification reasons, to avoid using void* types.
If your system is 32-bit, then pointer size will be 32-bits, which will fit the ULONG.

You can therefore use the casting, or pass the index, and retrieve the parameter via other API function passing index to it.

@parmi93
Copy link
Author

parmi93 commented Oct 1, 2024

This explanation doesn't convince me.
If they wanted to avoid using void * types for certification reasons, why did they use VOID * type for the next param stack_start?

@MaJerle
Copy link

MaJerle commented Oct 2, 2024

In fact you are right. Then it might be "legacy".

There are other details to challenge too, such as missing const keyword for thread name. Ive asked for an update in the past, they claimed we need to keep no const for legacy. Clearly unjustified reason as ut would not break anything.

@parmi93
Copy link
Author

parmi93 commented Oct 2, 2024

This raises another question: How do I pass one or more parameters (more complex than a ULONG) to a thread when creating it?
Do I really have to use a message queue?

@MaJerle
Copy link

MaJerle commented Oct 2, 2024

You should cast your pointer to ULONG and retrieve it back. This you can do.
It is a generic issue for OS, not linked to threadx IMO.

@parmi93
Copy link
Author

parmi93 commented Oct 2, 2024

As far as I know in C there is no guarantee that an unsigned long can be converted to void * and vice versa.
There are platforms out there where the size of a pointer is larger than an unsigned long.
Also (I'm not 100% sure about this), could pointers and integers have a different representation in memory?

@MaJerle
Copy link

MaJerle commented Oct 2, 2024

As far as I know in C there is no guarantee that an unsigned long can be converted to void * and vice versa. There are platforms out there where the size of a pointer is larger than an unsigned long. Also (I'm not 100% sure about this), could pointers and integers have a different representation in memory?

As stated above, in general you are right, but if you system is 32-bits, then void* and ULONG are of the same type, so you are good to go.

@parmi93
Copy link
Author

parmi93 commented Oct 2, 2024

I think the "32-bit system" requirement is not enough to ensure safe conversion from void* to ULONG (and vice versa).
There are 32-bit architectures like, Intel Architecture 32-bit (commonly called i386), where addresses are 6 bytes (2 bytes segment number and 4 bytes offset within the segment).

I'm writing a code (portable across different platforms) where I need to pass multiple parameters to a thread during its creation, I guess I'm not the first to encounter this problem, what mechanism has ThreadX implemented to be able to do this?

@MaJerle
Copy link

MaJerle commented Oct 2, 2024

Then the best proposal is to start the thread passing the data index and retrieving the data by:

void* get_thread_data(ULONG index) {
   return data_pointers[index];
}

void
thread_func(ULONG value) {
    void* data = get_thread_data(value);
}

@parmi93
Copy link
Author

parmi93 commented Oct 2, 2024

I guess (as you already mentioned in your first answer) this is the best solution, although I don't like the idea of ​​a global data_pointers.

@parmi93 parmi93 changed the title Why ThreadX entry function use ULONG as entry input instead of a void*? Why ThreadX entry function use ULONG as entry input instead of a void*? Oct 2, 2024
@parmi93 parmi93 changed the title Why ThreadX entry function use ULONG as entry input instead of a void*? Why ThreadX entry function use ULONG as entry input instead of a void *? Oct 2, 2024
@cypherbridge
Copy link

Hi we run across this often. The purpose of the formal parameter is to pass either a start code scalar integer, or alternatively an opaque pointer to a context structure. By convention ULONG is universal for 32 bit architectures and defined to be uint32_t, which also happens to be the size of a pointer. However on 64 bit machines the pointer will typically be allocated 8 bytes. For future updates it may be better to use stdint.h uintptr_t to pass an opaque pointer https://cplusplus.com/reference/cstdint/

@MaJerle
Copy link

MaJerle commented Oct 3, 2024

IMHO It should be void* and nothing else. Everything else is a workaround of a bad design.

@parmi93
Copy link
Author

parmi93 commented Oct 3, 2024

@cypherbridge As tx_thread_create() is currently designed, the input parameter is meant to accept only an integer, you should not pass an opaque pointer, because (as I mentioned before) there are 32-bit architectures out there, where the ULONG is 32-bit and pointers are 48-bit (which is perfectly legal for the C standard).

So the condition that must be satisfied in order to pass pointers is: sizeof(ULONG) >= sizeof(void *), the "32-bit architecture" condition is not sufficient, which in turn raises another question, what defines a 32-bit architecture? register size? address bus width? ALU size? something else? is there really a standard out there that defines what a 32-bit architecture is?

In any case, why uintptr_t and not a nice simple void *? which has been around since C was born, while uintptr_t has only existed since C99.
What advantages would it offer?

@hwmaier
Copy link

hwmaier commented Oct 3, 2024

It think there seems to be consensus that changing it to a portable pointer type makes sense. Maybe @parmi93 could create a respective PR?

In the mean time the opaque pointer cast can be considered a workaround for 32-bit architectures. The segmented 48-bit pointer case is hypothetical as to my knowledge ThreadX presently does not support such an architecture.

@cypherbridge
Copy link

C99 defines uintptr_t is defined in such a way to transparently convert between a pointer and and scalar.
With the benefit of cross-platform code, 32bit 64 bit, whatever.
Its true that in some projects ThreadX is used in C89 compiler which predates uintptr_t so for that reason
void* is a better choice and additionally similar to FreeRTOS xTaskCreate()
It seems the only use case we are talking about is tx_thread_create(), is there anything else affected by this?
Do keep in mind that whatever we come up with has the potential for broad impact to the installed code base, documentation, and so on. So what is the the rationale to do it?

@hwmaier
Copy link

hwmaier commented Oct 4, 2024

So what is the the rationale to do it?

I think the rationale is well explained by @parmi93 and I agree with the need for a mechanism to pass arbitrary context data to the thread in creation. This is common practise with all OS I have come across being it RTOSes or Linux or Windows and typically accomplished by passing a user defined pointer to the new thread. We are using this in our application as well and so far using the ULONG entry_input argument casted into from a pointer to a struct., but it would be better to change that to a void * pointer.

@hwmaier
Copy link

hwmaier commented Oct 4, 2024

And I agree this would be a breaking change, but sometimes in order to move forward you have to leave lagacy stuff behind. A ULONG was find when the world embraced 32-bit 20 years ago but now we have micros entering the 64-bit realm.

@agrutter
Copy link

agrutter commented Oct 4, 2024 via email

@parmi93
Copy link
Author

parmi93 commented Oct 4, 2024

C99 defines uintptr_t is defined in such a way to transparently convert between a pointer and and scalar.

I don't understand why ThreadX needs to bother allowing the user to convert a pointer to a scalar?

If entry_input had been a void * users would still have had the option to convert it to a uintptr_t later in entry_function() if they needed to.


Its true that in some projects ThreadX is used in C89 compiler which predates uintptr_t

I would also like to add that the uintptr_t type is optional in the c99 standard, and is still optional in the new c23 standard, so there might be some "modern compilers" that don't support uintptr_t at all.
https://port70.net/%7Ensz/c/c99/n1256.html#7.18.1.4


In any case I wasn't suggesting changing tx_thread_create(), I just wanted to understand why a ULONG was used instead of a void *, changing tx_thread_create() would break backwards compatibility (regardless of whether you use void * or uintptr_t).

I would recommend adding a new tx_thread_create2() that supports void *, leaving the old tx_thread_create() intact.

@MaJerle
Copy link

MaJerle commented Oct 4, 2024

I would recommend adding a new tx_thread_create2() that supports void *, leaving the old tx_thread_create() intact.

Or a threadx options where user can define the parameter type.

@parmi93
Copy link
Author

parmi93 commented Oct 4, 2024

Or a threadx options where user can define the parameter type.

This solution is even better, because it would not change the size of the program (binary) for those who want to continue using ULONG, the only thing users should be careful about is not using this option inappropriately, for example by defining the entry_input type as a giant struct, because this will be copied into the thread control block. But I guess we can't protect users from themselves.

@hwmaier
Copy link

hwmaier commented Oct 4, 2024

I like the option of being able to configure the type of entry_input, probably using a #define. This could default to ULONG and no backward compatibility issue arises then. Users can change it to be a VOID* or a struct ptr. Likewise the same could be done for the timer.

Having a second tx_thread_create2() with a new signature seems less ideal as more data needs to be added to the TX_THREAD_STRUCT structure and the the thread start code in _tx_thread_shell_entry() has to check which entry function to use, the old signature or the new signature. But its doable as well.

@xray-bit
Copy link

xray-bit commented Oct 9, 2024

A pointer points to an address of memory. The address of the memory is also a ULONG-compatible type.

@MaJerle
Copy link

MaJerle commented Oct 9, 2024

A pointer points to an address of memory. The address of the memory is also a ULONG-compatible type.

Not the case.

@parmi93
Copy link
Author

parmi93 commented Oct 9, 2024

The address of the memory is also a ULONG-compatible type.

This is not true, please read the entire discussion above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants