[FFmpeg-devel] "OPW Qualification Task: Enable daemon mode for FFserver"

[Binathi Bingi]

Hi there,

I am sorry for the indentation errors in the above mail, it was because of
bad email agent.

In the attached patch, if the config.logfilename is not "-", then stdout is
closed and then I reopened.

Reynaldo, is right. I don't think it is good idea to use dup2().
There were problems in running ffserver it couldn't start when I redirected
the file descriptor to stdin, stdout, stderr using dup2().
I removed dup2() and just opened something neutral in "/dev/null"
In this patch, I included error check for return value of open().

Cheers,
Binathi

On Tue, Nov 4, 2014 at 7:23 AM, Reynaldo H. Verdejo Pinochet <
reynaldo at osg.samsung.com> wrote:

> Hi
>
> On 11/03/2014 04:09 PM, Binathi Bingi wrote:
> > Hello,
> >
> > Inside the child process, I closed the file descriptor and then reopened
> > and redirected them using dup2() and later closed the opened file. I am
> not
> > sure if I understood and used the functionality of dup2() in the right
> > sense.
> >
>
> Looks about right but see bellow.
>
> > [..]
> >> @@ -3736,10 +3737,42 @@ int main(int argc, char **argv)
> >>      build_feed_streams();
> >>
> >>      compute_bandwidth();
> >> -
> >> +
>
> Remove trailing withe space
>
> >> +    if (config.ffserver_daemon) {
>
> What follows needs to be indented
>
> >> +    pid_t ffserver_id = 0;
> >> +    pid_t sid = 0;
> >> +
> >> +    ffserver_id = fork();
> >> +
> >> +    if (ffserver_id < 0) {
> >> +            ret = AVERROR(errno);
> >> +        av_log(NULL, AV_LOG_ERROR, "Impossible to start in daemon
> mode: %s\n", av_err2str(ret));
> >> +        exit(1);
>
> Fix wrong indentation
>
> >> +    }
> >> +
> >> +    if (ffserver_id > 0) {
> >> +        exit(0);
> >> +    }
>
> Drop the braces from single statement ifs blocks like this one.
>
> >> +
> >> +    sid = setsid();
> >> +    if (sid < 0) {
> >> +        exit(1);
> >> +    }
>
> Same as above
>
> >> +
> >> +    if (strcmp(config.logfilename, "-") != 0) {
> >> +            close(0);
>
> Drop the != 0. Anything but 0 will evaluate to true anyway. You do this
> elsewhere on your own code. Be consistent.
>
> >> +
> >> +    fd = open("/dev/null", O_RDWR);
> >> +    dup2(fd,0);
> >> +    dup2(fd,1);
> >> +    dup2(fd,2);
>
> You sure you wana dup2() stdin, stdout and stderr on the above
> condition? Please double check.
>
> >> +    close(fd);
>
> Above block needs to be re-indented. Also, check for failures in
> open and dup2() and react accordingly. I think Nicolas mentioned
> this already.
>
> >> +
> >> +        }
> >> +    }
> >>      /* signal init */
> >>      signal(SIGPIPE, SIG_IGN);
> >> -
> >> +
>
> Again, drop the trailing white space you introduced here and on
> multiple lines in this same patch.
>
> It's getting there. Congrats ;)
>
> Bests,
>
> --
> Reynaldo H. Verdejo Pinochet
> Open Source Group
> Samsung Research America / Silicon Valley
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Restore-Daemon-mode-in-FFserver.patch
Type: text/x-patch
Size: 4243 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20141105/89be3730/attachment.bin>