On Sun, Oct 02, 2016 at 02:30:11PM -0700, Philip Guenther wrote:
> On Sun, 2 Oct 2016, Rafael Zalamena wrote:
> > This diff is an improvement and an attempt to fix the bug where the
> > ntpd(8) not always stays running.
> >
> > During the review of syslogd fork+exec diff I noticed the use of dup3()
> > and went to read its man page: dup2() doesn't always remove the CLOEXEC
> > flag from the descriptor, so using dup3() is a better idea because it
> > does check the flag and if the oldd == newd then it retuns an useful
> > error.
> >
> > So if dup3() returns us an error it means oldd == newd and we should
> > remove the CLOEXEC flag ourself.
>
> IMO, it's better to just check for oldd==newd ourselves instead of making
> a call that we can tell will fail and then inferring the condition from
> the errno.
>
Thank you for the input, it does indeed look better, but I changed the
fatal() messages a little.
ok?
Index: util.c
===================================================================
RCS file: /home/obsdcvs/src/usr.sbin/ntpd/util.c,v
retrieving revision 1.22
diff -u -p -r1.22 util.c
--- util.c 14 Sep 2016 13:20:16 -0000 1.22
+++ util.c 2 Oct 2016 21:39:15 -0000
@@ -16,6 +16,7 @@
* OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
+#include <fcntl.h>
#include <limits.h>
#include <stdio.h>
#include <stdlib.h>
@@ -184,7 +185,11 @@ start_child(char *pname, int cfd, int ar
break;
case 0:
/* Prepare the parent socket and execute. */
- dup2(cfd, PARENT_SOCK_FILENO);
+ if (cfd != PARENT_SOCK_FILENO) {
+ if (dup2(cfd, PARENT_SOCK_FILENO) == -1)
+ fatal("%s: dup2", __func__);
+ } else if (fcntl(cfd, F_SETFD, 0) == -1)
+ fatal("%s: fcntl", __func__);
execvp(argv[0], nargv);
fatal("%s: execvp", __func__);
