Optional Unix Socket disable for devices that do not support it

[jeronimo-schreyer]

No description provided.

[akien-mga]

Would you mind providing more context in your PRs? This doesn't make sense, this define is not exposed anywhere in the buildsystem and nobody will ever know about it.

What platforms are you concerned about? How are you passing this define, and how should other porters learn of its existence?

[punto-]

It's for consoles :) Not all platforms have posix or posix-like sockets, this is just an option to disable the code like we have for other drivers, with the default set to build everywhere like it is now

[akien-mga]

It's for consoles :) Not all platforms have posix or posix-like sockets, this is just an option to disable the code like we have for other drivers, with the default set to build everywhere like it is now

Alright. There should be a comment to explain that next to the define, otherwise from the perspective of this open source repository, it's a useless check that someone may remove in the future when cleaning up.

[punto-]

We can add the comment, but these should be standard in all the drivers, not something that gets cleaned up, there are platforms that don't have gles or vulkan, or the windows audio driver, etc. That's why we had defines like this in all the driver code

edit: typo for comment :p

[akien-mga]

I asked for a comment, not a command :)

And yes, other drivers use similar checks, but we also have open source platforms defining them in their detect.py. If a define is only used on proprietary code we don't have access to, and not documented, how am I supposed to know that it's useful and for what?

[akien-mga]

For example:

$ rg PTHREAD_ENABLED
drivers/unix/semaphore_posix.cpp
33:#if defined(UNIX_ENABLED) || defined(PTHREAD_ENABLED)

drivers/unix/thread_posix.h
34:#if (defined(UNIX_ENABLED) || defined(PTHREAD_ENABLED)) && !defined(NO_THREADS)

drivers/unix/mutex_posix.h
34:#if defined(UNIX_ENABLED) || defined(PTHREAD_ENABLED)

drivers/unix/thread_posix.cpp
34:#if (defined(UNIX_ENABLED) || defined(PTHREAD_ENABLED)) && !defined(NO_THREADS)

drivers/unix/rw_lock_posix.cpp
31:#if defined(UNIX_ENABLED) || defined(PTHREAD_ENABLED)

drivers/unix/mutex_posix.cpp
35:#if defined(UNIX_ENABLED) || defined(PTHREAD_ENABLED)

drivers/unix/rw_lock_posix.h
34:#if defined(UNIX_ENABLED) || defined(PTHREAD_ENABLED)

drivers/unix/semaphore_posix.h
36:#if defined(UNIX_ENABLED) || defined(PTHREAD_ENABLED)

Is PTHREAD_ENABLED used for thirdparty ports? Or is it just a leftover from past implementations and now we just expect UNIX_ENABLED == PTHREAD_ENABLED?

[punto-]

That makes sense, "UNIX" means a lot of things, a lot of platforms have some parts of "unix" (like fopen or sockets), and not others (like pthreads), so we need granularity for sure. You don't know in advance when a platform will have something or not, so you provide the macro to let the platforms disable the code if they need to

[punto-]

Also I understand that some features are more standard than others, that's why we made it "enabled by default", so we don't need to go to every platform and enable this back

[akien-mga]

You misunderstand my point, I'm not saying that these defines don't make sense, I'm saying that in this codebase, they're never actually defined, so I have no means to know if they're dead code or if closed ports are actually using them.

PTHREAD_ENABLED was an example of such a define, which is queried but never set in this codebase, as shown above. Do you set it in a console port? How do I know?

[punto-]

But this basic rule of the codebase. When you make driver code, you have a define to disable it, otherwise you need to reference specific platforms outside of their platform/ code, in #ifdefs or if: blocks in scons, we don't want that. Maybe we need to write down these rules :) but it's been like that since the beginning for these situations where you don't know if someone will use it or not, but it's there just in case

[akien-mga]

Well I give up...