System V signal API using the standard POSIX API

Question

I wrote a program that implements a POSIX signal API. It implements the functions sigset(), sighold(), sigrelse(), sigignore() and sigpause().

I would like you to criticize my implementation, to find any problems that may arise.

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

#define SIG_HOLD ((__sighandler_t)2)
typedef void (*sighandler_t)(int);

sighandler_t
sigset(int sig, sighandler_t disp);
int sighold(int sig);
int sigrelse(int sig);
int sigignore(int sig);
int sigpause(int sig);

sighandler_t
sigset(int sig, sighandler_t disp)
{
    struct sigaction old_set;
    if (disp == SIG_HOLD)
    {
        sighold(sig);

        sigaction(sig, NULL, &old_set);
        return old_set.sa_handler;
    }

    struct sigaction act;
    sigemptyset(&act.sa_mask);
    act.sa_flags = 0;
    act.sa_handler = disp;
    sigaction(sig, &act, &old_set);
    return old_set.sa_handler;
}

int sighold(int sig)
{
    sigset_t set;
    sigemptyset(&set);
    sigaddset(&set, sig);
    return sigprocmask(SIG_BLOCK, &set, NULL);
}

int sigrelse(int sig)
{
    sigset_t set;
    sigemptyset(&set);
    sigaddset(&set, sig);
    return sigprocmask(SIG_UNBLOCK, &set, NULL);
}

int sigignore(int sig)
{
    if (signal(sig, SIG_IGN) == SIG_ERR)
        return -1;
    return 0;
}

int sigpause(int sig)
{
    sigset_t old_set;
    if (sigprocmask(SIG_BLOCK, NULL, &old_set) == -1)
        return -1;
    sigdelset(&old_set, sig);
    return sigsuspend(&old_set);
}

void handler(int sig)
{
    printf("handler\n"); // it's unsafe
}

int main()
{
    sigset(SIGTERM, handler);
    sighold(SIGABRT);
    sigrelse(SIGABRT);
    sigignore(SIGABRT);
    sighold(SIGTERM);
    raise(SIGTERM);
    sigpause(SIGTERM);

    return 0;
}

Answer 1

Unsafe definition of SIG_HOLD

There are several issues with your definition of SIG_HOLD. First, since you #include <signal.h>, consider that this macro might already be defined there. Use #ifndef SIG_HOLD to only define it if it doesn't already exist.

Second, there are other macros named SIG_*, which might have the same value as you are using for SIG_HOLD. To avoid any issues, I would recommend making sure your SIG_HOLD has a unique value. This is easy to do, just create a dummy function that you can take the address of:

#ifndef SIG_HOLD
void sig_hold_(int) {}
#define SIG_HOLD (sig_hold_)
#endif

Missing error checking

It is interesting that you check or pass on the return value of sigprocmask(), but you don't check the return value of most other sig*() functions. However, all of them return an error code that you should check.

Avoid using signal()

You are using both sigaction() and signal() in your code. While using signal(sig, SIG_IGN) might be one of the few uses of signal() that is safe, I would not use it and only rely on sigaction(). Note that you can just do:

int sigignore(int sig) {
    return sigset(sig, SIG_IGN);
}

Split it into a header file and a source file

You have everything in one file now, but you should try to put all these functions in a separate .c file, and create a matching .h file that can be #included in the source files that do want to use these functions. Also make sure you #include the header file in the .c file containing your functions; this avoids needing forward declarations, and can catch mistakes like the declarations in the header file not matching the definitions in the source file.