0

I want to delete the FIFO file when I suddenly click "ctrl+c" . I want to catch that signal and then delete the after before actually killing the process . here is my code and I don't know what went wrong :

 #include <fcntl.h>
 #include <stdio.h>
 #include  <stdlib.h>
 #include <sys/stat.h>
 #include <unistd.h>
 #include <signal.h>
 #define MAX_BUF 512

 void sigintHandler(int sig_num,char * myfifo)
{
puts("hello you there .please don't leave ");
unlink(myfifo);
exit(0);
}

int main()
{

printf("Access-Control-Allow-Origin: *\n");
printf("Content-Type: text/event-stream\n\n");
int fd;
char buf[MAX_BUF];

char * myfifo = "/tmp/omcipipe";
struct stat st;

//catch ctrl c

 sigaction(SIGINT, sigintHandler);



  //Create FIFO file 
     if (stat(myfifo, &st) != 0)
     mkfifo(myfifo, 0666);

 fd = open(myfifo, O_RDONLY);
while (read(fd, buf, MAX_BUF)>0)
{
    printf("data: %s", buf);
    printf("\n");
    fflush(stdout);
} 

puts( "----closing----"); 
close(fd);
unlink(myfifo);
return 0;

}

  • 4
    For one, you can't safely call functions that aren't async-signal-safe in a signal handler. Assuming Linux, [`puts()` and `exit()` are not async-signal-safe](http://man7.org/linux/man-pages/man7/signal-safety.7.html). – Andrew Henle May 02 '18 at 15:11
  • 1
    `void sigintHandler(int sig_num,char * myfifo)` is not valid, only an `int` (the signal number) is passed. – cdarke May 02 '18 at 15:20
  • 1
    Read [signal(7)](http://man7.org/linux/man-pages/man7/signal.7.html) & [signal-safety(7)](http://man7.org/linux/man-pages/man7/signal-safety.7.html). FIFOs and signals are operating system specific. You probably need some `linux` or `POSIX` tag – Basile Starynkevitch May 02 '18 at 15:28
  • And enable your compiler warnings - and pay attention to them. `sigaction(SIGINT, sigintHandler);` is even more wrong that I noticed at first - `sigaction()` is passed a signal number and two `struct sigaction *` values, not a signal number and a `void (*handler)(int)` signal handler function directly. – Andrew Henle May 02 '18 at 15:35
  • So I get from your comment is that I should not use "sigintHandler" in order for me to delete my FIFO file ? @Basile Starynkevitch – Oussema Ben Abdallah May 02 '18 at 15:48

2 Answers2

0

Firstly your way of setting a signal handler using sigaction() is not correct as you didn't fill all the member of struct sigaction. Fill all required member

struct sigaction info;
info.sa_handler = sigintHandler;
sigaction(SIGINT,&info,NULL);

Secondly, to read data from fifo file while loop is not required as you are reading MAX_BUF at a time. Loop is not required, read like this

int ret = read(fd, buf, MAX_BUF);
buf[ret-1] = '\0'; /*it should be null terminated */ 

Thirdly, sigintHandler() excepts only one argument. From the manual page of sigaction()

struct sigaction {
        void     (*sa_handler)(int);  /*handler expects only 1 argument */
        /* other member */
}

Finally & most importantly it is not safe to call functions like printf() & exit() from within a signal handler.

your sigHandler() looks like

static void sigintHandler(int sig_num) {
        write(1,"in isr\n",strlen("in isr\n"));
        /*  set some flag variable here & use that 
        flag variable in main() function to remove the fifo */
}

see this How to avoid using printf in a signal handler?

Here is the example code

static int fifo_flag = 0;
static void sigintHandler(int sig_num) {
        write(1,"in isr\n",strlen("in isr\n"));
        fifo_flag = 1;
}
int main(void){
        printf("Access-Control-Allow-Origin: *\n");
        printf("Content-Type: text/event-stream\n\n");
        int fd = 0, index = 0;
        char buf[MAX_BUF];
        #if 0
        char *myfifo = "data";
        #endif
        //char * myfifo = "/tmp/omcipipe";
        struct stat st;
        struct sigaction info;
        info.sa_handler = sigintHandler;
        //info.sa_flags = /* set to defaulgs a/c to your requirement*/

        if (stat(myfifo, &st) != 0) {
                mkfifo(myfifo, 0666);
                perror("mkfifo");
        }
        fd = open(myfifo, O_RDONLY | 0666);
        if(fd == -1){
                perror("open");
                return 0;
        }
        char ch = 0;
        while(read(fd, &ch, 1) > 0) {
                sigaction(SIGINT,&info,NULL);/* if ctrl+c  is called */
                buf[index] = ch;
                //sleep(1);/* just to observe ctrl+c behaviour, not advised to use */
                printf("%c\n",buf[index]);
                index++;
        }
        buf[index] = '\0';
        printf("data: %s", buf);
        printf("\n");
        puts( "----closing----");
        close(fd);
        if(fifo_flag == 1) { /*if flag is set, unlink fifo */
                unlink(myfifo); /* you can unlink fifo file here */
        }
        return 0;
}
Achal
  • 11,821
  • 2
  • 15
  • 37
0

There are numerous problems with your code.

This code assumes read() terminates data with a '\0' character:

while (read(fd, buf, MAX_BUF)>0)
{
    printf("data: %s", buf);
    printf("\n");
    fflush(stdout);
}

read() merely reads raw bytes and nothing more. It will not properly terminate strings with a '\0' character. So the printf() will almost certainly invoke undefined behavior.

This code

sigaction(SIGINT, sigintHandler);

is just wrong. sigaction() takes three parameters:

   #include <signal.h>

   int sigaction(int signum, const struct sigaction *act,
                 struct sigaction *oldact);

You also can only make async-signal-safe function calls from within a signal handler. This code

 void sigintHandler(int sig_num,char * myfifo)
{
puts("hello you there .please don't leave ");
unlink(myfifo);
exit(0);
}

calls puts() and exit(), neither of which are async-signal-safe.

And as noted in the comments, the signal handler code assumes that myfifo is passed as the second parameter. It isn't. Per the sigaction man page the second parameter is a struct siginfo * that contains information regarding the signal context - but only if the struct sigaction passed to the sigaction() call that registered the signal handler had the SA_SIGINFO flag set, and the handler set using the sa_sigaction member instead of the sa_handler member.

Andrew Henle
  • 32,625
  • 3
  • 24
  • 56