Skip to content

[RFC] posix: signal: empty + fill + set #60057

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

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions include/zephyr/posix/signal.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,62 @@
extern "C" {
#endif

#ifdef CONFIG_POSIX_SIGNAL
enum {
SIGABRT = 1,
SIGALRM,
SIGBUS,
SIGCHLD,
SIGCONT,
SIGFPE,
SIGHUP,
SIGILL,
SIGINT,
SIGKILL,
SIGPIPE,
SIGQUIT,
SIGSEGV,
SIGSTOP,
SIGTERM,
SIGTSTP,
SIGTTIN,
SIGTTOU,
SIGUSR1,
SIGUSR2,
SIGPOLL,
SIGPROF,
SIGSYS,
SIGTRAP,
SIGURG,
SIGVTALRM,
SIGXCPU,
SIGXFSZ,
};

/* Signal set management definitions and macros. */
#define MIN_SIGNO 1 /* Lowest valid signal number */
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 might move some of these into the enum above

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm somewhat concerned that using an ENUM will allow pre-processor defines to override these without generating a warning.

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe I shouldn't be lazy and should have used defines instead..

Copy link
Member

Choose a reason for hiding this comment

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

I think technically you can do both 😁 but some people might find that a bit too verbose

Copy link
Member

Choose a reason for hiding this comment

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

Generally though, #defines would give you more compile-time constness which is A Good Thing™️ for size.

#define MAX_SIGNO CONFIG_POSIX_SIGNAL_MAX /* Highest valid signal number */

/* Definitions for "standard" signals */
#define SIGSTDMIN 1 /* First standard signal number */
#define SIGSTDMAX CONFIG_POSIX_SIGNAL_STD_MAX /* Last standard signal number */

/* Definitions for "real time" signals */
#define SIGRTMIN (SIGSTDMAX + 1) /* First real time signal */
#define SIGRTMAX MAX_SIGNO /* Last real time signal */
#define _NSIG (MAX_SIGNO + 1) /* Biggest signal number + 1 */
#define NSIG _NSIG /* _NSIG variant commonly used */
Copy link
Member

Choose a reason for hiding this comment

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

are _NSIG, NSIG, MAX_SIGNO, SIGSTDMAX, SIGSTDMIN, etc standard names? I haven't looked yet.

Copy link
Member Author

Choose a reason for hiding this comment

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

_NSIG, NSIG

I can't find this in the standard, but pretty common in implementations to have these for calculation:

https://github.com/torvalds/linux/blob/master/include/uapi/asm-generic/signal.h#L7


/* sigset_t is represented as an array of 32-b unsigned integers.
* _SIGSET_NELEM is the allocated size of the array
*/
#define _SIGSET_NELEM ((_NSIG + 31) >> 5)
Copy link
Member

@cfriedt cfriedt Jul 5, 2023

Choose a reason for hiding this comment

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

are these standard symbols?

Normally underscore followed by capital letter is reserved.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not really standard, different implementations seems to have different names for this, to calculate the uint32_t array size to fit the number of signals, in nuttx its _SIGSET_NELEM, in Linux its _NSIG_WORDS


#define _SIGSET_NDX(s) ((s) >> 5) /* Get array index from signal number */
#define _SIGSET_BIT(s) ((s) & 0x1f) /* Get bit number from signal number */
#define _SIGNO2SET(s) ((uint32_t)1 << _SIGSET_BIT(s))
#endif /* CONFIG_POSIX_SIGNAL */

#ifndef SIGEV_NONE
#define SIGEV_NONE 1
#endif
Expand Down Expand Up @@ -41,6 +97,16 @@ typedef struct sigevent {
#endif
} sigevent;

#ifdef CONFIG_POSIX_SIGNAL
typedef struct sigset {
uint32_t _elem[_SIGSET_NELEM];
} sigset_t;

int sigaddset(sigset_t *set, int signo);
int sigemptyset(sigset_t *set);
int sigfillset(sigset_t *set);
#endif /* CONFIG_POSIX_SIGNAL */

#ifdef __cplusplus
}
#endif
Expand Down
1 change: 1 addition & 0 deletions lib/posix/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ zephyr_library_sources_ifdef(CONFIG_POSIX_FS fs.c)
zephyr_library_sources_ifdef(CONFIG_EVENTFD eventfd.c)
zephyr_library_sources_ifdef(CONFIG_FNMATCH fnmatch.c)
add_subdirectory_ifdef(CONFIG_GETOPT getopt)
add_subdirectory_ifdef(CONFIG_POSIX_SIGNAL signal)

zephyr_library_include_directories(
${ZEPHYR_BASE}/kernel/include
Expand Down
1 change: 1 addition & 0 deletions lib/posix/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# SPDX-License-Identifier: Apache-2.0

source "lib/posix/getopt/Kconfig"
source "lib/posix/signal/Kconfig"

config POSIX_MAX_FDS
int "Maximum number of open file descriptors"
Expand Down
7 changes: 7 additions & 0 deletions lib/posix/signal/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# SPDX-License-Identifier: Apache-2.0

zephyr_library_sources(
addset.c
emptyset.c
fillset.c
)
23 changes: 23 additions & 0 deletions lib/posix/signal/Kconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Copyright (c) 2023 Meta
# SPDX-License-Identifier: Apache-2.0


menuconfig POSIX_SIGNAL
bool "POSIX signal support"
default y if POSIX_API
help
This option adds support of POSIX signal.

config POSIX_SIGNAL_MAX
int "Highest valid signal number"
depends on POSIX_SIGNAL
default 63
help
Defines the highest valid signal number.

config POSIX_SIGNAL_STD_MAX
int "Highest number for standard signal"
depends on POSIX_SIGNAL
default 31
help
Defines the highest number for standard signal.
20 changes: 20 additions & 0 deletions lib/posix/signal/addset.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* Copyright (c) 2023 Meta
*
* SPDX-License-Identifier: Apache-2.0
*/

#include <signal.h>
#include <errno.h>

int sigaddset(sigset_t *set, int signo)
{
if ((set == NULL) || (signo <= 0) || (signo >= NSIG)) {
errno = EINVAL;
return -1;
}

set->_elem[_SIGSET_NDX(signo)] |= _SIGNO2SET(signo);

return 0;
}
16 changes: 16 additions & 0 deletions lib/posix/signal/emptyset.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/*
* Copyright (c) 2023 Meta
*
* SPDX-License-Identifier: Apache-2.0
*/

#include <signal.h>

int sigemptyset(sigset_t *set)
{
for (int ndx = 0; ndx < _SIGSET_NELEM; ndx++) {
set->_elem[ndx] = 0;
}

return 0;
}
17 changes: 17 additions & 0 deletions lib/posix/signal/fillset.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/*
* Copyright (c) 2023 Meta
*
* SPDX-License-Identifier: Apache-2.0
*/

#include <signal.h>
#include <stdint.h>

int sigfillset(sigset_t *set)
{
for (int ndx = 0; ndx < _SIGSET_NELEM; ndx++) {
set->_elem[ndx] = UINT32_MAX;
}

return 0;
}
10 changes: 10 additions & 0 deletions tests/posix/signal/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# Copyright (c) 2023, Meta
#
# SPDX-License-Identifier: Apache-2.0

cmake_minimum_required(VERSION 3.20.0)
find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE})
project(signal)

FILE(GLOB app_sources src/*.c)
target_sources(app PRIVATE ${app_sources})
5 changes: 5 additions & 0 deletions tests/posix/signal/Kconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Copyright (c) 2023, Meta
#
# SPDX-License-Identifier: Apache-2.0

source "Kconfig.zephyr"
3 changes: 3 additions & 0 deletions tests/posix/signal/prj.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
CONFIG_ZTEST=y
CONFIG_ZTEST_NEW_API=y
CONFIG_POSIX_API=y
41 changes: 41 additions & 0 deletions tests/posix/signal/src/addset.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright (c) 2023 Meta
*
* SPDX-License-Identifier: Apache-2.0
*/

#include <signal.h>
#include <errno.h>

#include <zephyr/ztest.h>
#include <zephyr/sys/util.h>
#include <zephyr/sys/util_macro.h>

ZTEST(posix_signal_apis, test_posix_signal_addset)
{
sigset_t set;
int signo;
int rc;

for (int i = 0; i < ARRAY_SIZE(set._elem); i++) {
set._elem[i] = 0u;
}

signo = 21;
zassert_ok(sigaddset(&set, signo));
zassert_equal(set._elem[signo >> 5], BIT(signo), "Signal %d is not set", signo);

signo = 42;
zassert_ok(sigaddset(&set, signo));
zassert_equal(set._elem[21 >> 5], BIT(21), "Signal %d is not set", 21);
zassert_equal(set._elem[signo >> 5], BIT(signo % (8 * sizeof(set._elem[0]))),
"Signal %d is not set", signo);

rc = sigaddset(&set, 0);
zassert_equal(rc, -1, "rc should be -1, not %d", rc);
zassert_equal(errno, EINVAL, "errno should be EINVAL, not %d", errno);

rc = sigaddset(&set, NSIG);
zassert_equal(rc, -1, "rc should be -1, not %d", rc);
zassert_equal(errno, EINVAL, "errno should be EINVAL, not %d", errno);
}
25 changes: 25 additions & 0 deletions tests/posix/signal/src/emptyset.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* Copyright (c) 2023 Meta
*
* SPDX-License-Identifier: Apache-2.0
*/

#include <zephyr/ztest.h>
#include <zephyr/sys/util.h>

#include <signal.h>

ZTEST(posix_signal_apis, test_posix_signal_emptyset)
{
sigset_t set;

for (int i = 0; i < ARRAY_SIZE(set._elem); i++) {
set._elem[i] = 0xffffffffu;
}

zassert_ok(sigemptyset(&set));

for (int i = 0; i < ARRAY_SIZE(set._elem); i++) {
zassert_equal(set._elem[i], 0, "signal is not empty");
}
}
26 changes: 26 additions & 0 deletions tests/posix/signal/src/fillset.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* Copyright (c) 2023 Meta
*
* SPDX-License-Identifier: Apache-2.0
*/

#include <zephyr/ztest.h>
#include <zephyr/sys/util.h>
#include <stdint.h>

#include <signal.h>

ZTEST(posix_signal_apis, test_posix_signal_fillset)
{
sigset_t set;

for (int i = 0; i < ARRAY_SIZE(set._elem); i++) {
set._elem[i] = 0u;
}

zassert_ok(sigfillset(&set));

for (int i = 0; i < ARRAY_SIZE(set._elem); i++) {
zassert_equal(set._elem[i], UINT32_MAX, "signal is not filled");
}
}
9 changes: 9 additions & 0 deletions tests/posix/signal/src/main.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/*
* Copyright (c) 2023, Meta
*
* SPDX-License-Identifier: Apache-2.0
*/

#include <zephyr/ztest.h>

ZTEST_SUITE(posix_signal_apis, NULL, NULL, NULL, NULL, NULL);