Thanks Anders for your comments.

Hi Hans, Ravi,

Is there any comment you would like to add, otherwise I update the patch with Anders' comments.

Thanks,

Minh


On 16/03/18 01:41, Anders Widell wrote:
Ack with minor comments, marked AndersW> below.

regards,

Anders Widell


On 03/15/2018 07:50 AM, Minh Chau wrote:
---
  src/base/os_defs.c | 25 ++++++++++++++++++++-----
  1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/src/base/os_defs.c b/src/base/os_defs.c
index 6f9ec52..e914011 100644
--- a/src/base/os_defs.c
+++ b/src/base/os_defs.c
@@ -1052,14 +1052,29 @@ uint32_t ncs_os_process_execute_timed(NCS_OS_PROC_EXECUTE_TIMED_INFO *req)
           * child */
          if (getenv("OPENSAF_KEEP_FD_OPEN_AFTER_FORK") == NULL) {
              /* Close all inherited file descriptors */
-            int i = sysconf(_SC_OPEN_MAX);
-            if (i == -1) {
+            int fd_max = sysconf(_SC_OPEN_MAX);

AndersW> sysconf() returns a long. Maybe fd_max should have the type long to match the return type of sysconf()?

+
+            if (fd_max == -1) {
                  syslog(LOG_ERR, "%s: sysconf failed - %s",
-                       __FUNCTION__, strerror(errno));
+                    __FUNCTION__, strerror(errno));
                  exit(EXIT_FAILURE);
              }
-            for (i--; i >= 0; --i)
-                (void)close(i); /* close all descriptors */
+            struct dirent *pentry = NULL;

AndersW> pentry is not a good name (avoid abbreviations, and separate words with underscores). Maybe rename it to dir_entry or just entry?

+            DIR *dir = opendir("/proc/self/fd");
+
+            if (dir != NULL) {
+                while ((pentry = readdir(dir)) != NULL) {
+                    int fd = strtoimax(pentry->d_name, NULL, 10);

AndersW> strtoimax() is declared in the inttypes.h header file. Add an #include at the top of the file. AndersW> strtoimax() returns an intmax_t. Change the type of fd to intmax_t.

+                    if (fd > INT_MIN && fd < fd_max) (void)close(fd);

AndersW> File descriptors cannot be negative. Use fd >= 0 instead of fd > INT_MIN.
AndersW> Remove (void).

+                }
+                (void)closedir(dir);

AndersW> Remove (void).

+            } else {
+                /* fall back, close all possible descriptors */
+                syslog(LOG_ERR, "%s: opendir failed - %s",
+                    __FUNCTION__, strerror(errno));
+                for (fd_max--; fd_max >= 0; --fd_max)
+                    (void)close(fd_max);

AndersW> Remove (void).

+            }
                /* Redirect standard files to /dev/null */
              if (freopen("/dev/null", "r", stdin) == NULL)




------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to