[PATCH 1/1] libc: Added implementations and prototypes for
Corinna Vinschen
vinschen@redhat.com
Mon Jul 19 09:47:59 GMT 2021
Hi Matt,
thanks for this implementation. The patch looks bascically fine.
A few issues, though.
First, it's missing the Makefile.am entries to build the new files,
please add them.
On Jul 17 12:10, Matt Joyce wrote:
> Added implementations for sig2str() and str2sig() in libc/signal in order
> to improve POSIX compliance. Added function prototypes to sys/signal.h.
> ---
> newlib/libc/include/sys/signal.h | 12 ++
> newlib/libc/signal/sig2str.c | 235 +++++++++++++++++++++++++++++++
> 2 files changed, 247 insertions(+)
> create mode 100644 newlib/libc/signal/sig2str.c
>
> diff --git a/newlib/libc/include/sys/signal.h b/newlib/libc/include/sys/signal.h
> index 45cc0366c..847dc59bd 100644
> --- a/newlib/libc/include/sys/signal.h
> +++ b/newlib/libc/include/sys/signal.h
> @@ -238,6 +238,18 @@ int sigqueue (pid_t, int, const union sigval);
>
> #endif /* __POSIX_VISIBLE >= 199309 */
>
> +#if __GNU_VISIBLE
> +
> +/* POSIX Issue 8 adds sig2str() and str2sig(). */
> +
> +/* This allows for the max length of the error message and longest integer. */
> +#define SIG2STR_MAX sizeof("Unknown signal 4294967295 ")
While looking through your patch I realized that this request of mine
was nonsense. The idea was that the buffers of length SIG2STR_MAX
have to store descriptive signal texts, like "Floating point exception"
or "Real-time signal 10", but that's not the case here anyway.
If SIG2STR_MAX isn't already defined, gnulib defines SIG2STR_MAX
basically as sizeof "SIGRTMAX" + sizeof (max numerical string of type
int). This results in different SIG2STR_MAX values depending of sizeof
int being 2, 4, or 8.
Given we strive for supporting smaller targets we might want to express
this similary, just a bit simpler. For one thing, do we ever want to
support more than 4 billion RT signals? I guess not. My suggestion would
be something like:
#if __STDINT_EXP(INT_MAX) > 0x7fff
#define SIG2STR_MAX (sizeof ("RTMAX+") + sizeof ("4294967295") - 1)
#else
#define SIG2STR_MAX (sizeof ("RTMAX+") + sizeof ("65535") - 1)
#endif
> +int
> +sig2str(int signum, char *str)
> +{
> + const sig_name_and_num *sptr;
> +
> + /* If signum falls in the real time signals range, the Issue 8 standard
> + * gives the option of defining the saved str value as either "RTMIN+n" or
> + * "RTMAX-m".*/
> + if ((SIGRTMIN + 1) <= signum && signum <= (SIGRTMAX -1)) {
^^^
space
> + sprintf(str, "RTMIN+%d", (signum-SIGRTMIN));
> + return 0;
> + }
This is not entirely correct. The POSIX draft requires the signal
string to be returned for RT signals to be expressed as either "RTMIN+x"
and "RTMAX-x", dependent on the signal number. Please see
https://www.opengroup.org/austin/docs/austin_1110.pdf, page 85, the two
paragraphs starting at line 61678 (unfortunately copy/paste from this
doc doesn't work nicely).
> +int
> +str2sig(const char *restrict str, int *restrict pnum)
> +{
> + int j = 0;
> + const sig_name_and_num *sptr;
> + char dest[SIG2STR_MAX];
> + int is_valid_decimal;
> + is_valid_decimal = atoi(str);
> +
> + /* If str is a representation of a decimal value, save its integer value
> + * in pnum. */
> + if (1 <= is_valid_decimal && is_valid_decimal <= SIGRTMAX) {
> + *pnum = is_valid_decimal;
> + return 0;
> + }
> +
> + /* If str is in RT signal range, get number of of RT signal, save it as an
> + * integer. The Issue 8 standard requires */
^^^
Looks truncated?
> + if (strncmp(str, "RTMIN+", SPACES_TO_N) == 0) {
> + j = atoi(&str[SPACES_TO_N]);
Oops... strncmp returns 0, but you didn't check if the next char is
actually a non-NUL char, or even a digit. Only digits are valid, so
atoi should be replaced with strtoul with an extra check for endptr.
> + /* If number is valid, save it in pnum. */
> + if (1 <= j && j <= ((SIGRTMAX - SIGRTMIN)-1)) {
^^^
spaces
We have a special problem here. i686 Cygwin only supports a single RT
signal. For historical reasons, don't ask.
I. e., SIGRTMIN == SIGRTMAX. This special case should be checked here,
to make sure the code works with this very special, very non-POSIX
behaviour. I think the easiest way to handle that is to skip the
entire "RTMIN+"/"RTMAX-" code if SIGRTMIN == SIGRTMAX. There just is
no such valid value in this case.
> + *pnum = (SIGRTMIN + j);
> + return 0;
> + }
> + return -1;
> + }
> +
> + /* If str is in RT signal range, get number of of RT signal, save it as an
> + * integer. */
> + if (strncmp(str, "RTMAX-", SPACES_TO_N) == 0) {
> + j = atoi(&str[SPACES_TO_N]);
Ditto.
> + /* If number is valid, save it in pnum. */
> + if (1 <= j && j <= ((SIGRTMAX - SIGRTMIN)-1)) {
^^^
spaces
Thanks,
Corinna
More information about the Newlib
mailing list