-
Notifications
You must be signed in to change notification settings - Fork 1k
utils.c: include <signal.h> for siginfo_t
#7517
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
Conversation
POSIX says: > The <signal.h> header shall define the siginfo_t type as a structure So <sys/wait.h> is not enough to see the definition (not just a forward declaration) of siginfo_t.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7517 +/- ##
=======================================
Coverage 99.02% 99.02%
=======================================
Files 87 87
Lines 16803 16803
=======================================
Hits 16640 16640
Misses 163 163 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Generated via commit 3cc6658 Download link for the artifact containing the test results: ↓ atime-results.zip
|
I have used |
MichaelChirico
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great!
|
Ironically, setting We use Unlike |
|
I'm just looking for a way to fortify our checks such that we would have caught this issue before release |
|
Then the strict checks must use
Should I push f02dcb7 to this branch? Drat. R itself uses configure-time tests for C23 features in |
|
Since we need to patch |
* utils.c: include <signal.h> for siginfo_t POSIX says: > The <signal.h> header shall define the siginfo_t type as a structure So <sys/wait.h> is not enough to see the definition (not just a forward declaration) of siginfo_t. * NEWS entry * Amend NEWS * more robustly define _POSIX_C_SOURCE (h/t Hugh) * tidy up NEWS * -D_POSIX_C_SOURCE=200809L in gitlab CI job for regression test * revert gitlab-ci change --------- Co-authored-by: Michael Chirico <[email protected]> Co-authored-by: Michael Chirico <[email protected]>
|
Also |
|
|
||
| 6. `rowwiseDT()` now provides a helpful error message when a complex object that is not a list (e.g., a function) is provided as a cell value, instructing the user to wrap it in `list()`, [#7219](https://github.com/Rdatatable/data.table/issues/7219). Thanks @kylebutts for the report and @venom1204 for the fix. | ||
|
|
||
| 7. Fixed compilation failure like "error: unknown type name 'siginfo_t'" in v1.18.0 in some strict environments, e.g., FreeBSD, where the header file declaring the POSIX function `waitid` does not transitively include the header file defining the `siginfo_t` type, [#7516](https://github.com/rdatatable/data.table/issues/7516). Thanks to @jszhao for the report and @aitap for the fix. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very minor: the underscore in "error: unknown type name 'siginfo_t'" is an unpaired emphasis mark. This doesn't confuse Pandoc or commonmark, but makes the Vim syntax highlighter unhappy. Some variation on "error: unknown type name 'siginfo\_t'" or "error: unknown type name `siginfo_t`" would be slightly better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer `error: unknown type name 'siginfo_t'` to keep it human-readable too

POSIX says:
So
<sys/wait.h>is not enough to see the definition (not just a forward declaration) ofsiginfo_t.Fixes: #7516
Tested manually on FreeBSD:
Details
...and OpenBSD:
Details
Also tested on GNU/Linux with
PKG_CFLAGS=--std=c99.@jszhao, could you please pull from this branch (
posix_siginfo_t) and runR CMD build, then install the resulting tarball?