We need to be careful that all file descriptors have the FD_CLOEXEC 
flag, then. It can easily happen that someone later on creates a file 
descriptor without this flag, without knowing that it is needed.

If we had a debug build, then we could have a loop surronded by #ifdef 
DEBUG, that checks that all file descriptors have the FD_CLOEXEC flag. 
Then we would easily be able to catch any such regression.

/ Anders Widell

2014-04-23 09:56, Hans Feldt skrev:
> Yes we know that for sure, it is amfnd or smfnd calling this routine.
>
>  From what I remember one closeonexec is needed (some leap fd) then we can 
> remove the close loop and the reopen part. I think we should aim for that 
> instead of patching.
>
> /HansF
>
>> -----Original Message-----
>> From: Anders Widell
>> Sent: den 23 april 2014 09:54
>> To: Hans Feldt; Hans Nordebäck; ramesh.bet...@oracle.com
>> Cc: opensaf-devel@lists.sourceforge.net
>> Subject: Re: [PATCH 1 of 1] base: use dup2 instead of freopen (non 
>> async-signal-safe) V1 [#532]
>>
>> In that case the child will have the same stdin/stdout/stderr as the
>> parent process. But if we know for sure that the parent already has
>> redirected its stdin/stdout/stderr to /dev/null then I think it ought to
>> be fine.
>>
>> / Anders Widell
>>
>> 2014-04-23 09:45, Hans Feldt skrev:
>>> What happens in/with the child process if this code is instead removed 
>>> completely?
>>> /HansF
>>>
>>>> -----Original Message-----
>>>> From: Hans Nordebäck
>>>> Sent: den 23 april 2014 09:11
>>>> To: Hans Feldt; Anders Widell; ramesh.bet...@oracle.com
>>>> Cc: opensaf-devel@lists.sourceforge.net
>>>> Subject: [PATCH 1 of 1] base: use dup2 instead of freopen (non 
>>>> async-signal-safe) V1 [#532]
>>>>
>>>>    osaf/libs/core/leap/os_defs.c |  21 +++++++++++++++------
>>>>    1 files changed, 15 insertions(+), 6 deletions(-)
>>>>
>>>>
>>>> New issues where freopen "freezes", use dup2 instead.
>>>> Non async-signal-safe are used in ncs_os_process_execute_timed,
>>>> sched_setscheduler, syslog, setenv, getenv and freopen. Syslog may
>>>> "freeze" but are called only when an error has been detected. The
>>>> remaining should be removed.
>>>>
>>>> diff --git a/osaf/libs/core/leap/os_defs.c b/osaf/libs/core/leap/os_defs.c
>>>> --- a/osaf/libs/core/leap/os_defs.c
>>>> +++ b/osaf/libs/core/leap/os_defs.c
>>>> @@ -971,6 +971,7 @@ uint32_t ncs_os_process_execute_timed(NC
>>>>
>>>>                    /* By default we close all inherited file descriptors 
>>>> in the child */
>>>>                    if (getenv("OPENSAF_KEEP_FD_OPEN_AFTER_FORK") == NULL) {
>>>> +                  int fd1;
>>>>                            /* Close all inherited file descriptors */
>>>>                            int i = sysconf(_SC_OPEN_MAX);
>>>>                            if (i == -1) {
>>>> @@ -981,12 +982,20 @@ uint32_t ncs_os_process_execute_timed(NC
>>>>                                    (void) close(i); /* close all 
>>>> descriptors */
>>>>
>>>>                            /* Redirect standard files to /dev/null */
>>>> -                  if (freopen("/dev/null", "r", stdin) == NULL)
>>>> -                          syslog(LOG_ERR, "%s: freopen stdin failed - 
>>>> %s", __FUNCTION__, strerror(errno));
>>>> -                  if (freopen("/dev/null", "w", stdout) == NULL)
>>>> -                          syslog(LOG_ERR, "%s: freopen stdout failed - 
>>>> %s", __FUNCTION__, strerror(errno));
>>>> -                  if (freopen("/dev/null", "w", stderr) == NULL)
>>>> -                          syslog(LOG_ERR, "%s: freopen stderr failed - 
>>>> %s", __FUNCTION__, strerror(errno));
>>>> +                  if ((fd1 = open("/dev/null", O_RDWR)) < 0) {
>>>> +                          syslog(LOG_ERR, "%s: open /dev/null failed - 
>>>> %s", __FUNCTION__, strerror(errno));
>>>> +                  }
>>>> +                  else {
>>>> +                          if (dup2(fd1, STDIN_FILENO) < 0) {
>>>> +                                  syslog(LOG_ERR, "%s: dup2 stdin failed 
>>>> - %s", __FUNCTION__, strerror(errno));
>>>> +                          }
>>>> +                          if (dup2(fd1, STDOUT_FILENO) < 0) {
>>>> +                                  syslog(LOG_ERR, "%s: dup2 stdout failed 
>>>> - %s", __FUNCTION__, strerror(errno));
>>>> +                          }
>>>> +                          if (dup2(fd1, STDERR_FILENO) < 0) {
>>>> +                                  syslog(LOG_ERR, "%s: dup2 stderr failed 
>>>> - %s", __FUNCTION__, strerror(errno));
>>>> +                          }
>>>> +                  }
>>>>                    }
>>>>
>>>>                    /* RUNASROOT gives the OpenSAF user a possibility to 
>>>> maintain the < 4.2 behaviour.



------------------------------------------------------------------------------
Start Your Social Network Today - Download eXo Platform
Build your Enterprise Intranet with eXo Platform Software
Java Based Open Source Intranet - Social, Extensible, Cloud Ready
Get Started Now And Turn Your Intranet Into A Collaboration Platform
http://p.sf.net/sfu/ExoPlatform
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to