This should have been folded into the other patch so we reviewed a
final version.
On Wed, Jul 7, 2021 at 5:47 AM Matt Joyce <mfjoyce2...@gmail.com> wrote:
>
> Added features to function implementations to conform with POSIX standard.
> ---
> newlib/libc/signal/sig2str.c | 96 ++++++++++++++++++++++++++++++------
> 1 file changed, 81 insertions(+), 15 deletions(-)
>
> diff --git a/newlib/libc/signal/sig2str.c b/newlib/libc/signal/sig2str.c
> index a07fa2a38..152d10cf7 100644
> --- a/newlib/libc/signal/sig2str.c
> +++ b/newlib/libc/signal/sig2str.c
> @@ -1,9 +1,14 @@
> /* Placeholder */
> -//#define __GNU_VISIBLE // defining it to have access to SIG2STR_MAX
>
> +/* Defining _GNU_SOURCE to have access to SIG2STR_MAX in signal.h. */
> +#define _GNU_SOURCE
> #include <signal.h>
> #include <string.h>
> #include <stdio.h>
> +#include <stdlib.h>
> +
> +#define SPACES_TO_N 6
> +#define NUM_OF_SIGS (sizeof(sig_array) / sizeof(sig_name_and_num))
>
> typedef struct sig_name_and_num {
> const char *sig_name;
> @@ -101,12 +106,12 @@ static sig_name_and_num sig_array[] = {
> #ifdef SIGUSR2
> { "USR2", SIGUSR2 },
> #endif
> - // #ifdef SIGRTMIN
> - // { "RTMIN", SIGRTMIN },
> - // #endif
> - // #ifdef SIGRTMAX
> - // { "RTMAX", SIGRTMAX },
> - // #endif
> + #ifdef SIGRTMIN
> + { "RTMIN", SIGRTMIN },
> + #endif
> + #ifdef SIGRTMAX
> + { "RTMAX", SIGRTMAX },
> + #endif
Why is this in here? You won't get it as input. RT signals get processed
special AFAIK looking at other implementations.
> #ifdef SIGPWR
> { "PWR", SIGPWR },
> #endif
> @@ -127,25 +132,86 @@ static sig_name_and_num sig_array[] = {
> #endif
> };
>
> -#define NUM_OF_SIGS (sizeof(sig_array) / sizeof(sig_name_and_num))
> -
> int
> sig2str(int signum, char *str)
> {
> + /* Real Time Signals, lower half */
I just don't understand the upper or lower half. It is RTSIGnn and the
nn is what you are computing.
> + if ((SIGRTMIN + 1) <= signum && signum <= (SIGRTMIN + SIGRTMAX) / 2){
I think this should be written >= and <= to indicate a range. I don't
think this idiom is used frequently.
> + sprintf(str, "RTMIN+%d", (signum-SIGRTMIN));
> + return 0;
> + }
>
> - for (sig_name_and_num *i = sig_array; i < &sig_array[NUM_OF_SIGS]; i++){
> - if (i->sig_num == signum){
> + /* Real Time Signals, upper half */
> + else if ((((SIGRTMIN +SIGRTMAX)/ 2) + 1) <= signum && signum <= \
Why the line break?
Why the splitting of the range? I think RT signals are guaranteed to
be consecutive
between MIN and MAX so a simple (signum >= SIGRTMIN && signum <= SIGRTMAX)
should be sufficient.
> + (SIGRTMAX - 1)){
> + sprintf(str, "RTMAX-%d", (SIGRTMAX-signum));
> + return 0;
> + }
Spacing on ){
> +
> + /* All others, including SIGRTMIN / SIGRTMAX */
> + else{
You don't need an else if the RT signals were processed above. Something like:
if (signum >= SIGRTMIN && signum <= SIGRTMAX) {
/* process it */
return 0;
}
and then fall into the other loop.
> + for (sig_name_and_num * i = sig_array; i < &sig_array[NUM_OF_SIGS]; i++){
> + if (i->sig_num == signum){
> strcpy(str, i->sig_name);
> return 0;
> - }
> + }
> + }
> + sprintf(str, "Unknown signal %d", signum);
> + return -1;
> }
> - sprintf(str, "Unknown signal %d", signum);
> - return -1;
> }
>
> int
> -str2sig(const char *__restrict str, int *__restrict pnum)
> +str2sig(const char *restrict str, int *restrict pnum)
Why not use the __restrict like the rest of newlib?
> {
> + int j = 0;
> + char dest[SIG2STR_MAX];
> + int 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;
> + }
Is this if condition right?
And "is_valid_decimal" seems an odd name. It is converting to a number.
If invalid, deal with it. If valid, it needs to be in range.
> +
> + /* If str is in RT signal range, get number of of RT signal, save it as an
> + * integer. */
of of
> + if (strncmp(str, "RTMIN+", SPACES_TO_N) == 0){
> + for(int i = SPACES_TO_N; str[i] != '\0'; i++){
> + dest[j] = str[i];
> + j++;
> + }
> + dest[j] = '\0';
> + j = atoi(dest);
> +
> + /* If number is valid, save it in pnum. */
> + if (1 <= j && j <= ((SIGRTMAX - SIGRTMIN)-1)){
> + *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){
> + for(int i = SPACES_TO_N; str[i] != '\0'; i++){
> + dest[j] = str[i];
> + j++;
> + }
> + dest[j] = '\0';
> + j = atoi(dest);
> +
> + /* If number is valid, save it in pnum. */
> + if (1 <= j && j <= ((SIGRTMAX - SIGRTMIN)-1)){
> + *pnum = (SIGRTMAX - j);
> + return 0;
> + }
> + return -1;
> + }
> +
> + /*If str is a valid signal name, save its corresponding number in pnum. */
> for (sig_name_and_num *i = sig_array; i < &sig_array[NUM_OF_SIGS]; i++){
> if (strcmp(i->sig_name, str) == 0){
> *pnum = i->sig_num;
Watch the formatting.
> --
> 2.31.1
>
> _______________________________________________
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
