From 29d833fdd8b0c64475160eddebf6cb5a40ab489d Mon Sep 17 00:00:00 2001 From: gumartinm Date: Sun, 20 Jan 2013 22:28:54 +0100 Subject: [PATCH] Multiple minor changes: 1. Return -1 from main function as error code. 2. Close useless file descriptors in child process. 3. Improving some comments and error messages. 4. Errors from poll function. --- Daemon/javafork.c | 52 +++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 37 insertions(+), 15 deletions(-) diff --git a/Daemon/javafork.c b/Daemon/javafork.c index 7d0a42b..5500539 100644 --- a/Daemon/javafork.c +++ b/Daemon/javafork.c @@ -60,7 +60,7 @@ int main (int argc, char *argv[]) /*This process is intended to be used as a daemon, it sould be launched by the INIT process, because of that*/ /*we are not forking it (INIT needs it)*/ if (daemonize(argv[0], LOG_SYSLOG, LOG_PID) < 0) - return 1; + return -1; /*Changing session.*/ setsid(); @@ -76,7 +76,7 @@ int main (int argc, char *argv[]) pvalue = atoi(optarg); if ((pvalue > 65535) || (pvalue <= 0)) { syslog (LOG_ERR, "Port value %d out of range", pvalue); - return 1; + return -1; } break; case 'q': @@ -89,7 +89,7 @@ int main (int argc, char *argv[]) syslog (LOG_ERR, "Invalid option '-%c'.", optopt); else syslog (LOG_ERR, "Unknown option character '\\x%x'.", optopt); - return 1; + return -1; default: abort (); } @@ -98,7 +98,7 @@ int main (int argc, char *argv[]) /*This program does not admit options*/ if (optind < argc) { syslog (LOG_ERR,"This program does not admit options just argument elements with their values."); - return 1; + return -1; } @@ -111,7 +111,7 @@ int main (int argc, char *argv[]) memset (&sigintAction, 0, sizeof(sigaction)); if (sigaction (SIGINT, NULL, &sa) < 0) { syslog (LOG_ERR, "SIGINT retrieve current signal handler failed: %m"); - return 1; + return -1; } if (sa.sa_handler != SIG_IGN) { @@ -122,17 +122,17 @@ int main (int argc, char *argv[]) sa.sa_flags = SA_RESTART; if (sigemptyset(&sa.sa_mask) < 0) { syslog (LOG_ERR, "SIGINT empty mask: %m"); - return 1; + return -1; } if (sigaction(SIGINT, &sa, NULL) < 0) { syslog (LOG_ERR, "SIGINT set signal handler failed: %m"); - return 1; + return -1; } } if (main_daemon (avalue, pvalue, qvalue) < 0) - return 1; + return -1; return 0; } @@ -246,7 +246,8 @@ int daemonize(const char *pname, int facility, int option) closeSafely(fd); return 0; } - + + /*TODO: Errors from syslog function*/ /*Sending messages to log*/ openlog(pname, option, facility); @@ -415,7 +416,7 @@ int readable_timeout (int fd, long timeout, long utimeout) ret = TEMP_FAILURE_RETRY(select(fd+1, &fd_read, NULL, NULL, &ptime)); if (ret == 0) { - syslog(LOG_INFO, "receiving timeout error"); + syslog(LOG_INFO, "receiving timeout error: %m"); return -1; } else if (ret == -1) { syslog(LOG_ERR, "receiving error: %m"); @@ -556,6 +557,7 @@ int fork_system(int socket, unsigned char *command, int *returnstatus) int returnValue = 0; /*return value from this function can be caught by upper layers,*/ /*OK by default*/ sigset_t unBlockMask; /*Used by the child process in order to unblock the SIGCHLD signal, which was blocked by the main process.*/ + int pollReturn; /*Value by default*/ @@ -578,8 +580,7 @@ int fork_system(int socket, unsigned char *command, int *returnstatus) if (pid == 0) { /*Child process*/ - /*It has to launch another one using system or execve*/ - /*TODO: close every fd but 0, 1, 2 (or my pipes) and the read end of my pipes.*/ + /*It has to launch another one using system or better directly using execve*/ /*Unblock SIGCHLD*/ if (sigemptyset(&unBlockMask) < 0) { @@ -606,6 +607,13 @@ int fork_system(int socket, unsigned char *command, int *returnstatus) _exit(EXIT_FAILURE); } + /*Close useless file descriptors. The child inherits copies of the parent's set of open file descriptors. See as well: CLOEXEC*/ + closeSafely (out[0]); + closeSafely (out[1]); + closeSafely (err[0]); + closeSafely (err[1]); + closeSafely (sockfd); + closeSafely (socket); /*TODO: I should use execve with setlocale and the environment instead of system.*/ /*During execution of the command, SIGCHLD will be blocked, and SIGINT and SIGQUIT*/ @@ -624,12 +632,15 @@ int fork_system(int socket, unsigned char *command, int *returnstatus) /*Parent process*/ /*It sends data to the Java client using a TCP connection.*/ /*TODO: close the write end of my pipes.*/ + + /*Attach pipes' read end to pollfd*/ polls[0].fd=out[0]; polls[1].fd=err[0]; polls[0].events = polls[1].events = POLLIN; for (;;) { - if(poll(polls, 2, 100)) { + pollReturn = poll(polls, 2, 100); + if(pollReturn > 0) { if(polls[0].revents && POLLIN) { bzero(buf,2000); n=TEMP_FAILURE_RETRY(read(out[0], &buf[sizeof(struct tcpforkhdr)], 2000-sizeof(struct tcpforkhdr))); @@ -662,7 +673,7 @@ int fork_system(int socket, unsigned char *command, int *returnstatus) header->type = htonl(2); header->length = htonl(n); if (TEMP_FAILURE_RETRY(send(socket, buf, n+sizeof(struct tcpforkhdr), 0)) < 0) - syslog (LOG_INFO, "error while sending stdout: %m"); + syslog (LOG_INFO, "error while sending stderr: %m"); } if(!(polls[0].revents && POLLIN) && !(polls[1].revents && POLLIN)) { @@ -687,7 +698,7 @@ int fork_system(int socket, unsigned char *command, int *returnstatus) goto err; } } - else { + else if (pollReturn == 0) { /*When timeout*/ if(TEMP_FAILURE_RETRY(waitpid(pid, &childreturnstatus, WNOHANG))) { //TODO: treating errors in waitpid function @@ -700,6 +711,17 @@ int fork_system(int socket, unsigned char *command, int *returnstatus) } /*The child process is not dead, keep polling more data from stdout or stderr streams*/ } + else { + /*Return with error from poll*/ + syslog (LOG_ERR, "poll error: %m"); + + /*Trying to kill child process*/ + if (kill (pid, SIGTERM) == -1) { + syslog (LOG_ERR, "kill error: %m"); + } + + goto err; + } } } /*Stuff just done by the parent process. The child process ends with exit*/ -- 2.1.4