bpo-31904: fix signalmodule issue in VxWorks

[yuesun1]

The type of sig_atomic_t is 'unsigned char' in VxWorks, so it can't be assigned to -1, so in the signalmodule.c. I set the data type of fd in struct 'wakeup' to 'int' for VxWorks.

More and full support on modules for VxWorks will continuously be added by the coming PRs.
VxWorks is a product developed and owned by Wind River. For VxWorks introduction or more details, go to https://www.windriver.com/products/vxworks/
Wind River will have a dedicated engineering team to contribute to the support as maintainers.
We already have a working buildbot worker internally, but has not bound to master. We will check the process for the buildbot, then add it.

https://bugs.python.org/issue31904

[jdemeyer]

The fd member is clearly meant to be a file descriptor, which is of type int (as specified by POSIX). So the old code is unconditionally wrong: you should use int on all systems, not just VxWorks. This happened to work because sig_atomic_t is int on almost all systems.

[jdemeyer]

I'd rather frame this as a general bug-fix and not a VxWorks-specific issue.

[jdemeyer]

Now that you're correctly using int, reading/writing it may no longer be atomic. So you should use the atomic macros to access it (there are plenty of examples in signalmodule.c doing that).

[yuesun1]

Thanks for your comment, now the fd is not atomic, do you mean I also still use _Py_atomic_load() or _Py_atomic_store() to read/write it ?

Thanks very much

[vstinner]

Please open a separated issue for this change.

[jdemeyer]

Thanks for your comment, now the fd is not atomic, do you mean I also still use _Py_atomic_load() or _Py_atomic_store() to read/write it ?

Yes, I think so. You'll find many uses of these functions in signalmodule.c.

[vstinner]

The easiest way to fix VxWorks but not impact other operating systems if to modify your PR as, pseudo-code:

#ifdef <VxWorks>
int ...
#else
sig_atomic_t ...
#endif

[jdemeyer]

The existing code is unconditionally wrong: it's a file descriptor, which is of type int.

[vstinner]

The existing code is unconditionally wrong: it's a file descriptor, which is of type int.

I already asked "Please open a separated issue for this change." but nobody did that.

[jdemeyer]

I already asked "Please open a separated issue for this change." but nobody did that.

I know but that doesn't imply that the existing code is correct. It only means that nobody cares enough to open an issue.

[vstinner]

It only means that nobody cares enough to open an issue.

In case of doubt, I'm ok to leave the code unchanged: that's the safest option. That's the principle of https://en.wikipedia.org/wiki/Wikipedia:Chesterton%27s_fence To remove the fence, someone must first investigate why it's there.

This issue is about VxWorks. I disagre that adding VxWorks support would change the code on other platforms.

[vstinner]

Let's continue the work on PR #23391.