Well lets remove the 3*open part then.
Thanks,
Hans

On 04/23/2014 12:11 PM, Ramesh Betham wrote:
> Me too have the same opinion, there is a good chance that 'fd' can be created 
> without FD_CLOEXEC flag.
> Well, 'close()' is an async-signal-safe function.. so it should not be a 
> problem.
>
> Regards,
> Ramesh.
>
> On 4/23/2014 1:43 PM, Anders Widell wrote:
>> 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