0

I wish to understand, if my implementation of signal handling in my code is correct

Some context

Some external application creates two pipes, which I will be connecting to, this external application has been correctly implemented (Guaranteed)

My two pipes are pe_exchange_0 and pe_trader_0 the {0} is passed in via command line args

Now, the external application is writing to the pe_exchange_0 pipe, and my code is suppose to be reading the contents it is writing into. The max size of the content it writes is 50 (so our char buffer needs to be of size 50).

Now after this is done, I need to send a SIGUSR1 from my application (the trader) to the exchange and write a message saying "Hello"

The process id of the exchange is the parent id so i can simply do kill(exchange_pid, SIGUSR1);

When sending a message to my exchange

Now here is a sample example

EXCHANGE (external application) -> Trader: Hi There!

my code does not detect the sigusr1 call and does not print out the message from the exchange

it should send back a subsequent echo

Trader(my app) -> Exchange(External) = Hello World

This is what I have so far.

    void signal_handler(int SIG);

volatile sig_atomic_t g_sigint_flag = 0;
volatile sig_atomic_t g_sigusr1_flag = 0;
int fd1;
int fd2;

void exchange_signal_handler(int signo);

int main(int argc, char ** argv) {

    char exchange[100];
    char trader[100];

    sprintf(exchange, "/tmp/pe_exchange_%s", argv[1]);
    sprintf(trader, "/tmp/pe_trader_%s", argv[1]);

    fd1 = open(exchange,O_RDONLY);
    fd2 = open(trader,O_WRONLY);
    if (fd1== -1) {
        perror("open pipe failed");
        return 1;
    }

    if (fd2== -1) {
        perror("open pipe failed");
        return 1;
    }

    if (signal(SIGINT, signal_handler) == SIG_ERR) {
        printf("Unable to register signal handler for SIGINT\n");
        return 1;
    }

    if (signal(SIGUSR1, exchange_signal_handler) == SIG_ERR) {
        printf("Unable to register exchange signal handler for SIGUSR1\n");
        return 1;
    }

    pid_t exchange_pid = getppid();

    while (1) {
        if (g_sigusr1_flag) {
            g_sigusr1_flag = 0;
            printf("This is working partially");// Reset the flag
        }
        sleep(1);
    }

    close(fd2);
    close(fd1);
    return 0;
}

void signal_handler(int SIG) {
    if (SIG == SIGINT) {
        printf("Terminated\n");
        g_sigint_flag = 1;
    }
}

void exchange_signal_handler(int signo) {
    if (signo == SIGUSR1) {
        printf("Received SIGUSR1 signal from exchange\n");
        g_sigusr1_flag = 1;
    }
}
  • Perhaps unrelated, but your signal handlers invoke undefined behaviour because `printf(3)` is not async-signal-safe. Use `write(2)` instead. – Harith Apr 25 '23 at 08:15
  • wdym by write(2) and printf(3) – HyperCoderSuperion Apr 25 '23 at 08:17
  • *"Re: My code should detect the sigusr1 call and print out the message from the exchange*" ===> But does it? Does it produce the expected output? – Harith Apr 25 '23 at 08:18
  • No it does not unfortunately – HyperCoderSuperion Apr 25 '23 at 08:23
  • Hence the question sorry I should of been clearer – HyperCoderSuperion Apr 25 '23 at 08:24
  • See https://man7.org/linux/man-pages/man7/signal-safety.7.html and https://man7.org/linux/man-pages/man2/write.2.html – Harith Apr 25 '23 at 09:33
  • See [How to avoid using `printf()` in a signal handler?](https://stackoverflow.com/q/16891019/15168) – Jonathan Leffler Apr 25 '23 at 15:05
  • Report which file (pipe — presumably these are FIFOs) can't be opened: `if ((fd1 = open(exchange, O_RDONLY)) < 0) { perror(exchange); return 1; }` would be better, though personally I'd prefer a more flexible error reporting function. You could use the [`err(3)`](http://linux.die.net/man/3/err) function on Linux: `if ((fd1 = open(exchange, O_RDONLY)) < 0) err(1, "failed to open file %s for reading", exchange);` for example. The `err()` function prints a message including the program name and the message for `errno` and exits with status `1` because that's what's passed as its first argument. – Jonathan Leffler Apr 25 '23 at 15:12
  • 1
    Use [`sigaction()`](https://pubs.opengroup.org/onlinepubs/9699919799/functions/sigaction.html) rather than `signal()` — see [What is the difference between `sigaction()` and `signal()`?](https://stackoverflow.com/q/231912/15168) for reasons why. – Jonathan Leffler Apr 25 '23 at 15:17
  • You should not normally reenable interrupts if they are being ignored, so the standard sequence (using `signal()` for laziness) is: `if (signal(SIGINT, SIG_IGN) != SIG_IGN) signal(SIGINT, signal_handler);`. The equivalent using `sigaction()` is a little more verbose. You should probably not enable the quit signal if is ignored. With other signals, YMMV. For example, if you use SIGUSR1 to communicate between processes, you should install your signal handler unconditionally. With `sigaction()`, you can find out if the signal is ignored without changing the handling — but it's still two calls. – Jonathan Leffler Apr 25 '23 at 15:19
  • You should check that `argv[1]` is set before using it — `if (argc != 1) { fprintf(stderr, "Usage: %s number\n", argv[0]); exit(EXIT_FAILURE); }`. Also, don't forget to end messages with a newline, and sometimes it's a good idea to use `fflush(stdout);` to help ensure messages do appear on standard output in a timely fashion. The code shown isn't using `fd1` or `fd2`. – Jonathan Leffler Apr 25 '23 at 15:24

1 Answers1

0

The process id of the exchange is the parent id so i can simply do kill(exchange_pid, SIGUSR1);

Yet, you don't do it.

Ale
  • 887
  • 10
  • 14