[PATCH 1/2] libc: Added implementations for sig2str/str2sig

[Matthew Joyce]

On Wed, Jul 7, 2021 at 4:04 PM Joel Sherrill <joel at rtems.org> wrote:
>
> On Wed, Jul 7, 2021 at 5:46 AM Matt Joyce <mfjoyce2004 at gmail.com> wrote:
> >
> > Added function implementations for sig2str() and str2sig() in libc/signal in order
> > to improve POSIX compliance.
> > ---
> >  newlib/libc/signal/sig2str.c | 156 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 156 insertions(+)
> >  create mode 100644 newlib/libc/signal/sig2str.c
> >
> > diff --git a/newlib/libc/signal/sig2str.c b/newlib/libc/signal/sig2str.c
> > new file mode 100644
> > index 000000000..a07fa2a38
> > --- /dev/null
> > +++ b/newlib/libc/signal/sig2str.c
> > @@ -0,0 +1,156 @@
> > +/* Placeholder  */
>
> Why placeholder? Seems like an odd comment.

I need to include a license / header block here...I think I should use our most
current RTEMS block, but I wanted to check to make sure!

> > +//#define __GNU_VISIBLE // defining it to have access to SIG2STR_MAX
> > +
> > +#include <signal.h>
> > +#include <string.h>
> > +#include <stdio.h>
> > +
> > +typedef struct sig_name_and_num {
> > +  const char *sig_name;
> > +  const int  sig_num;
> > +} sig_name_and_num;
> > +
> > +static sig_name_and_num sig_array[] = {
>
> This is also const.
>
Got it, thanks!

> > +    #ifdef EXIT
> > +    { "EXIT", 0 },
> > +    #endif
>
> I don't think this should be included. And I don't know where you saw it.

It was in the Solaris implementation...I'll take it out!

>
> > +    #ifdef SIGHUP
> > +    { "HUP", SIGHUP},
> > +    #endif
>
> Indent the middle line (include the ifdef)
>
> > +    #ifdef SIGINT
> > +    { "INT", SIGINT },
> > +    #endif
> > +    #ifdef SIGQUIT
> > +    { "QUIT", SIGQUIT },
> > +    #endif
> > +    #ifdef SIGILL
> > +    { "ILL", SIGILL },
> > +    #endif
> > +    #ifdef SIGTRAP
> > +    { "TRAP", SIGTRAP },
> > +    #endif
> > +    #ifdef SIGABRT
> > +    { "ABRT", SIGABRT },
> > +    #endif
> > +    #ifdef SIGIOT
> > +    { "IOT", SIGIOT},
> > +    #endif
> > +    #ifdef SIGEMT
> > +    { "EMT", SIGEMT },
> > +    #endif
> > +    #ifdef SIGFPE
> > +    { "FPE", SIGFPE },
> > +    #endif
> > +    #ifdef SIGKILL
> > +    { "KILL", SIGKILL },
> > +    #endif
> > +    #ifdef SIGBUS
> > +    { "BUS", SIGBUS },
> > +    #endif
> > +    #ifdef SIGSEGV
> > +    { "SEGV", SIGSEGV },
> > +    #endif
> > +    #ifdef SIGSYS
> > +    { "SYS", SIGSYS },
> > +    #endif
> > +    #ifdef SIGPIPE
> > +    { "PIPE", SIGPIPE },
> > +    #endif
> > +    #ifdef SIGALRM
> > +    { "ALRM", SIGALRM },
> > +    #endif
> > +    #ifdef SIGTERM
> > +    { "TERM", SIGTERM },
> > +    #endif
> > +    #ifdef SIGURG
> > +    { "URG", SIGURG },
> > +    #endif
> > +    #ifdef SIGSTOP
> > +    { "STOP", SIGSTOP },
> > +    #endif
> > +    #ifdef SIGTSTP
> > +    { "TSTP", SIGTSTP },
> > +    #endif
> > +    #ifdef SIGCONT
> > +    { "CONT", SIGCONT },
> > +    #endif
> > +    #ifdef SIGCHLD
> > +    { "CHLD", SIGCHLD },
> > +    #endif
> > +    #ifdef SIGCLD
> > +    { "CLD", SIGCLD },
> > +    #endif
> > +    #ifdef SIGTTIN
> > +    { "TTIN", SIGTTIN },
> > +    #endif
> > +    #ifdef SIGTTOU
> > +    { "TTOU", SIGTTOU },
> > +    #endif
> > +    #ifdef SIGIO
> > +    { "IO", SIGIO },
> > +    #endif
> > +    #ifdef SIGPOLL
> > +    { "POLL", SIGPOLL },
> > +    #endif
> > +    #ifdef SIGWINCH
> > +    { "WINCH", SIGWINCH },
> > +    #endif
> > +    #ifdef SIGUSR1
> > +    { "USR1", SIGUSR1 },
> > +    #endif
> > +    #ifdef SIGUSR2
> > +    { "USR2", SIGUSR2 },
> > +    #endif
> > +    // #ifdef SIGRTMIN
> > +    // { "RTMIN", SIGRTMIN },
> > +    // #endif
> > +    // #ifdef SIGRTMAX
> > +    // { "RTMAX", SIGRTMAX },
> > +    // #endif
> > +    #ifdef SIGPWR
> > +    { "PWR", SIGPWR },
> > +    #endif
> > +    #ifdef SIGXCPU
> > +    { "XCPU", SIGXCPU },
> > +    #endif
> > +    #ifdef SIGXFSZ
> > +    { "XFSZ", SIGXFSZ },
> > +    #endif
> > +    #ifdef SIGVTALRM
> > +    { "VTALRM", SIGVTALRM },
> > +    #endif
> > +    #ifdef SIGPROF
> > +    { "PROF", SIGPROF },
> > +    #endif
> > +    #ifdef SIGLOST
> > +    { "LOST", SIGLOST }
> > +    #endif
> > +};
> > +
>
> Is this the entire set used across all newlib signal.h files?

I've double checked and to my knowledge it has each one from signal.h
and sys/signal.h.
It does not contain __SIGFIRSTNOTRT and __SIGLASTNOTRT. I hesitated to include
those two because in sys/signal.h they were in an #ifdef and mapped to
the values of HUP
and SIGUSR2 respectively. I thought it would not make sense (in the
way I understood it)
to have them in that way, not knowing what would be defined.

Thinking about it, if they should be included, I could keep the former
as HUP, and define the
latter as (SIGRTMIN - 1), correct?

>
> > +#define NUM_OF_SIGS (sizeof(sig_array) / sizeof(sig_name_and_num))
> > +
> > +int
> > +sig2str(int signum, char *str)
> > +{
> > +
> > +  for (sig_name_and_num *i = sig_array; i < &sig_array[NUM_OF_SIGS]; i++){
>
> Declare the variable at the top of the function. I am not sure newlib
> has gone to assuming
> a new enough version of C for this to work on all target
> configurations. This is also in the
> other method.
>

Understood, thanks!

> > +    if (i->sig_num == signum){
>
> ) {
>
> needs a space. This occurs multiple times.
>
I'm sorry, I don't understand this one...do you mean the brackets? They
seem aligned in my file...or maybe I cleaned them up in the second patch?

> > +        strcpy(str, i->sig_name);
> > +        return 0;
> > +    }
> > +  }
> > +  sprintf(str, "Unknown signal %d", signum);
> > +  return -1;
> > +}
> > +
> > +int
> > +str2sig(const char *__restrict str, int *__restrict pnum)
> > +{
> > +  for (sig_name_and_num *i = sig_array; i < &sig_array[NUM_OF_SIGS]; i++){
>
> Dec
Ok, thank you.

> > +    if (strcmp(i->sig_name, str) == 0){
> > +        *pnum = i->sig_num;
> > +        return 0;
> > +    }
> > +  }
> > +  return -1;
> > +}
> > --
> > 2.31.1
> >
> > _______________________________________________
> > devel mailing list
> > devel at rtems.org
> > http://lists.rtems.org/mailman/listinfo/devel