Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

bpo-31904: fix signalmodule issue in VxWorks#12670

Closed
yuesun1 wants to merge 4 commits intopython:masterfrom
Wind-River:fix-issue-31904-signalmodule
Closed

bpo-31904: fix signalmodule issue in VxWorks#12670
yuesun1 wants to merge 4 commits intopython:masterfrom
Wind-River:fix-issue-31904-signalmodule

Conversation

@yuesun1
Copy link

@yuesun1yuesun1 commentedApr 3, 2019
edited by bedevere-bot
Loading

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 tohttps://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

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

@jdemeyerjdemeyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Thefd member is clearly meant to be a file descriptor, which is of typeint (as specified by POSIX). So the old code is unconditionally wrong: you should useint on all systems, not just VxWorks. This happened to work becausesig_atomic_t isint on almost all systems.

@@ -0,0 +1 @@
fix VxWorks signalmodule issue
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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

@jdemeyer
Copy link
Contributor

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

@yuesun1
Copy link
Author

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
Copy link
Member

Please open a separated issue for this change.

@jdemeyer
Copy link
Contributor

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 insignalmodule.c.

@brettcannonbrettcannon added the type-bugAn unexpected behavior, bug, or error labelApr 17, 2019
@vstinner
Copy link
Member

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

#ifdef <VxWorks>int ...#elsesig_atomic_t ...#endif

@jdemeyer
Copy link
Contributor

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

@vstinner
Copy link
Member

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
Copy link
Contributor

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
Copy link
Member

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 ofhttps://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
Copy link
Member

Let's continue the work on PR#23391.

@pxinwrpxinwr deleted the fix-issue-31904-signalmodule branchJuly 12, 2021 09:42
@kuhlenoughkuhlenoughmannequin mentioned this pull requestJan 12, 2024
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@jdemeyerjdemeyerjdemeyer requested changes

Assignees

No one assigned

Labels

awaiting core reviewtype-bugAn unexpected behavior, bug, or error

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@yuesun1@jdemeyer@vstinner@brettcannon@the-knights-who-say-ni@bedevere-bot

Comments


[8]ページ先頭

©2009-2026 Movatter.jp