Skip to content
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

posix: add stubs for signal.h functions that need process support #74436

Merged
merged 4 commits into from
Jun 27, 2024

Conversation

cfriedt
Copy link
Member

@cfriedt cfriedt commented Jun 17, 2024

Since Zephyr itself does not currently support processes, but conformant applications should still be able to link, add stubs for the remaining POSIX functions in the POSIX_SIGNALS Option Group.

The POSIX_SIGNALS Option Group is required for PSE51, PSE52, PSE53, PSE54, and likely many other POSIX Subprofiles.

Note

There is a Coding Guidelines false positive about declaring the raise() function (from C89).

Doc Preview

Fixes #59925
Fixes #59933
Fixes #66923
Fixes #66924
Fixes #66925
Fixes #66929
Fixes #66930

@cfriedt cfriedt force-pushed the posix-signals-stubs branch 2 times, most recently from a38404d to 2baac4a Compare June 17, 2024 18:15
@cfriedt cfriedt added area: POSIX POSIX API Library area: Documentation area: Tests Issues related to a particular existing or missing test labels Jun 17, 2024
@cfriedt cfriedt force-pushed the posix-signals-stubs branch from 2baac4a to 37370cf Compare June 17, 2024 19:29
@vaishnavachath vaishnavachath self-requested a review June 17, 2024 20:03
@ceolin ceolin self-requested a review June 17, 2024 23:18
ceolin
ceolin previously approved these changes Jun 17, 2024
char *strsignal(int signum);
int sigemptyset(sigset_t *set);
int sigfillset(sigset_t *set);
int sigaddset(sigset_t *set, int signo);
int sigdelset(sigset_t *set, int signo);
int sigismember(const sigset_t *set, int signo);
void (*signal(int signo, void (*)(int signo)))(int signo);
Copy link
Member

Choose a reason for hiding this comment

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

GNU extension is so much clear :)

typedef void (*sighandler_t)(int);

sighandler_t signal(int signum, sighandler_t handler);

Copy link
Member Author

Choose a reason for hiding this comment

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

It is convenient, but also a gnu-ism.

Copy link
Member

Choose a reason for hiding this comment

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

yep :)

Copy link
Member Author

Choose a reason for hiding this comment

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

I haven't seen any conflicts when using the typedef, even when building using native_sim, so I'll add it as well.

sigset_t sa_mask;
int sa_flags;
void (*sa_sigaction)(int signo, siginfo_t *info, void *context);
};
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't it be

struct sigaction {
	void (*sa_handler)(int signno);
	void (*sa_sigaction)(int signo, siginfo_t *info, void *context);
	sigset_t sa_mask;
	int sa_flags;
};

to avoid possible hole in the struct.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed!

@cfriedt cfriedt marked this pull request as ready for review June 17, 2024 23:51
@zephyrbot zephyrbot added the area: C Library C Standard Library label Jun 17, 2024
ycsin
ycsin previously approved these changes Jun 18, 2024
@cfriedt cfriedt added the DNM This PR should not be merged (Do Not Merge) label Jun 26, 2024
@cfriedt
Copy link
Member Author

cfriedt commented Jun 26, 2024

Adding a temporary DNM until all comments have been addressed.

@cfriedt cfriedt dismissed stale reviews from ycsin and ceolin via 2fe2dcd June 26, 2024 19:53
@cfriedt cfriedt force-pushed the posix-signals-stubs branch from 37370cf to 2fe2dcd Compare June 26, 2024 19:53
@cfriedt cfriedt requested review from ceolin and ycsin June 26, 2024 19:54
@cfriedt
Copy link
Member Author

cfriedt commented Jun 26, 2024

  • rebased
  • rearranged struct members to be more compact
  • added sighandler_t typedef

@cfriedt cfriedt removed the DNM This PR should not be merged (Do Not Merge) label Jun 26, 2024
@cfriedt
Copy link
Member Author

cfriedt commented Jun 26, 2024

Other than the false positive in coding guidelines, this should probably be all green. Please revisit when you have a moment @ceolin, @ycsin

ycsin
ycsin previously approved these changes Jun 26, 2024
Chris Friedt added 3 commits June 27, 2024 11:45
Newlib requires an alias for the getpid() function. There was
a Kconfig already present for doing so, but the actual alias
was missing.

So this is technically a bugfix.

Signed-off-by: Chris Friedt <[email protected]>
Since Zephyr itself does not currently support processes, but
conformant applications should still be able to link, add stubs
for the remaining POSIX functions in the POSIX_SIGNALS Option
Group.

The POSIX_SIGNALS Option Group is required for PSE51, PSE52,
PSE53, PSE54, and likely many other POSIX Subprofiles.

Signed-off-by: Chris Friedt <[email protected]>
Add tests for the presence of items in the POSIX SIGNALS Option
Group.

Signed-off-by: Chris Friedt <[email protected]>
@cfriedt
Copy link
Member Author

cfriedt commented Jun 27, 2024

  • rebased to resolve merge conflict

Please add a refresh +1 @ycsin @vaishnavachath

@cfriedt cfriedt requested a review from ycsin June 27, 2024 15:47
Mark the POSIX_SIGNALS Option Group as supported to the extent
possible under Zephyr, as Zephyr does not yet support processes.

Signed-off-by: Chris Friedt <[email protected]>
@cfriedt cfriedt force-pushed the posix-signals-stubs branch from e511bdd to 4bd5b2c Compare June 27, 2024 15:49
@nashif nashif merged commit b10f1ca into zephyrproject-rtos:main Jun 27, 2024
21 of 22 checks passed
@cfriedt cfriedt deleted the posix-signals-stubs branch June 27, 2024 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: C Library C Standard Library area: Documentation area: POSIX POSIX API Library area: Tests Issues related to a particular existing or missing test
Projects
None yet
5 participants