[patch] various popen.c cleanups

Sergio Gelato <[email protected]>
Newsgroups gmane.comp.sysutils.cfengine.bugs
Message-ID <[email protected]>
Like others, I've been bothered by the MAXFD messages. So I looked at
recent SVN history, picked up post-2.2.8 delta 594, and noticed some 
residual issues. Here are three patches, one per issue:

1) cfpclose() can and should pclose(pp) even after issuing the MAXFD warning.
2) *all* calls to pthread_mutex_unlock() need to be #ifdef'ed. Also fix
   a trivial cut-and-paste error in a CfLog() call.
3) don't let cfpopen() gratuitously fail when the MAXFD limit is exceeded:
   we've already successfully launched the child process, and there is now
   code in place to cfpwait() even when we've lost track of the child pid.

_______________________________________________
Bug-cfengine mailing list
[email protected]
https://cfengine.org/mailman/listinfo/bug-cfengine
cfengine-2.2.8-3.diff (text/x-diff, 390 B)
diff -ru old/src/popen.c new/src/popen.c
--- old/src/popen.c	2008-09-07 23:29:55.000000000 +0200
+++ new/src/popen.c	2008-09-07 23:32:14.358426000 +0200
@@ -722,8 +722,7 @@
       "MAXFD, check for defunct children", fd);
    CfLog(cferror,OUTPUT,"");
    /* We can't wait for the specific pid as it isn't stored in CHILD. */
-   cfpwait(-1);
-   return -1;
+   pid = -1;
    }
 else
    {
cfengine-2.2.8-4.diff (text/x-diff, 744 B)
diff -ru old/src/popen.c new/src/popen.c
--- old/src/popen.c	2008-09-07 23:32:14.000000000 +0200
+++ new/src/popen.c	2008-09-07 23:35:00.080215160 +0200
@@ -67,7 +67,7 @@
 #if defined HAVE_PTHREAD_H && (defined HAVE_LIBPTHREAD || defined BUILDTIN_GCC_THREAD)
 if (pthread_mutex_lock(&MUTEX_COUNT) != 0)
    {
-   CfLog(cferror,"pthread_mutex_unlock failed","pthread_mutex_unlock");
+   CfLog(cferror,"pthread_mutex_lock failed","pthread_mutex_lock");
    return NULL;
    }
 #endif
@@ -76,7 +76,9 @@
    {
    if ((CHILD = calloc(MAXFD,sizeof(pid_t))) == NULL)
       {
+#if defined HAVE_PTHREAD_H && (defined HAVE_LIBPTHREAD || defined BUILDTIN_GCC_THREAD)
       pthread_mutex_unlock(&MUTEX_COUNT);
+#endif
       return NULL;
       }
    }
cfengine-2.2.8-6.diff (text/x-diff, 1.4 KB)
diff -ru old/src/popen.c new/src/popen.c
--- old/src/popen.c	2008-09-07 23:38:34.000000000 +0200
+++ new/src/popen.c	2008-09-07 23:40:31.883798240 +0200
@@ -201,15 +201,12 @@
       {
       snprintf(OUTPUT,CF_BUFSIZE,"File descriptor %d of child %d higher than MAXFD, check for defunct children", fileno(pp), pid);
       CfLog(cferror,OUTPUT,"");
-      /* Don't leave zombies. */
-      cfpwait(pid);
-      return NULL;
       }
    else
       {
       CHILD[fileno(pp)] = pid;
-      return pp;
       }
+   return pp;
    }
 
 return NULL; /* Cannot reach here */
@@ -376,9 +373,6 @@
       {
       snprintf(OUTPUT,CF_BUFSIZE,"File descriptor %d of child %d higher than MAXFD, check for defunct children", fileno(pp), pid);
       CfLog(cferror,OUTPUT,"");
-      /* Don't leave zombies. */
-      cfpwait(pid);
-      return NULL;
       }
    else
       {
@@ -501,9 +495,6 @@
       {
       snprintf(OUTPUT,CF_BUFSIZE,"File descriptor %d of child %d higher than MAXFD, check for defunct children", fileno(pp), pid);
       CfLog(cferror,OUTPUT,"");
-      /* Don't leave zombies. */
-      cfpwait(pid);
-      return NULL;
       }
    else
       {
@@ -650,9 +641,6 @@
       {
       snprintf(OUTPUT,CF_BUFSIZE,"File descriptor %d of child %d higher than MAXFD, check for defunct children", fileno(pp), pid);
       CfLog(cferror,OUTPUT,"");
-      /* Don't leave zombies. */
-      cfpwait(pid);
-      return NULL;
       }
    else
       {
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.