Dangerous and invalid use of ThreadX internals with local path feature

[vlang-intona]

FileX isn't supposed have a hard dependency on ThreadX, yet it does, and it even depends on its internals, and it probably causes real bugs.

Example:

https://github.com/eclipse-threadx/filex/blob/master/common/src/fx_directory_local_path_set.c#L147

FileX uses _tx_thread_current_ptr, which is ThreadX-specific, and internal to ThreadX. It's so internal that ThreadX doesn't even expose it in its API headers, so FileX just defines it itself (what an excellent WTF):

https://github.com/eclipse-threadx/filex/blob/master/common/inc/fx_api.h#L225

I can't believe that ThreadX itself contains FileX-specific code to support this:

https://github.com/eclipse-threadx/threadx/blob/master/common/inc/tx_api.h#L552

As you can see, just accessing that _tx_thread_current_ptr global variable is probably not correct on the ThreadX SMP kernel:

https://github.com/eclipse-threadx/threadx/blob/master/common_smp/inc/tx_api.h#L1169
https://github.com/eclipse-threadx/threadx/blob/master/common_smp/src/tx_thread_identify.c#L96

How could it, there's probably no TLS support at all. This leads me to believe that using API like fx_directory_local_path_set() leads to undefined behavior (for managers: BIG $$$ LOSS) if used on ThreadX SMP builds. I can't believe I'm finding such nonsense and beginner level mistakes in a proven RTOS that prides itself on having received safety certification by TÜV. (Maybe FileX isn't meant to live up to the standards ThreadX adheres to, but there's still FileX-specific code in ThreadX...)

You can disable this feature with FX_NO_LOCAL_PATH, but this leads to further inconveniences. The local path feature has some justification (it's like a per-thread chdir()), but it should not depend on ThreadX internals. It shouldn't require ThreadX either. But what I would actually expect is a sane API like POSIX opendir().

[vlang-intona]

In addition I have to express my disbelief how whoever implemented this FileX feature put extra effort into doing it in a buggy way by messing with ThreadX internals, instead of just calling tx_thread_identify().

Seriously, how did this happen?

[fdesbiens]

Seriously, how did this happen?

Good question. I have been around for less than a year and am not aware of the full history. I will have a chat with more experienced team members when I have the chance.

By the way, thank you for sharing your feedback, @vlang-intona. Can you share more context about your standalone use of FileX? Given our limited resources, I cannot see myself putting a high priority on improving FileX outside of ThreadX right now. That said, we could consider merging patches that address the issues you describe.

[vlang-intona]

By the way, thank you for sharing your feedback, @vlang-intona. Can you share more context about your standalone use of FileX? Given our limited resources, I cannot see myself putting a high priority on improving FileX outside of ThreadX right now. That said, we could consider merging patches that address the issues you describe.

We are using it for an embedded project with relatively constrained firmware (otherwise we'd just run Linux on it). We use it with ThreadX on baremetal. In addition, we have a host port of the firmware for testing, which doesn't use ThreadX. I'm aware that ThreadX has host ports, but we considered them too complex and too hairy to have a real use for us. FileX porting wasn't that hard, but this was one of the issues that required dirty workarounds.

Anyway, there are the following arguments why the issue reported is an actual problem:

• It most likely breaks on the ThreadX SMP kernel (I have not tried it, but it looks like it)
• The API, which requires per-thread data internally, is clunkier than an API, which would not require this
• FileX seems to put effort into being not dependent on ThreadX normally

[fdesbiens]

Thank you for the additional context, @vlang-intona. I will discuss this with the team and get back to you.