[MPlayer-dev-eng] [PATCH] Use unrar for open vobsubs if available

[Ulion]

2007/12/7, Rich Felker <dalias at aerifal.cx>:
> On Fri, Dec 07, 2007 at 12:31:53PM +0800, Ulion wrote:
> > > This looks like a horrible idea, the next file that the unrar binary opens
> > > can get 2 as file descriptor, and any error messages will end up in some
> > > "random" file.
> >
> > Fixed, should the stdin be closed also?
>
> Probably doesn't hurt, as long as you dup /dev/null onto fd 0 and
> don't just leave it closed..

I mean is it safe to leave the stdin opened as current code did.

>
> > One thing to make using popen calls safer is to make sure the
> > executable is an existed file, then the shell will not search the
> > executable in its search PATH, and it also make the executable set by
> > user harder be some strange hack string if it possiblly be hacked.
>
> Using a name in the PATH is perfectly valid and not insecure. If
> someone is stupid enough to put . in their PATH it's their problem...
> Also I'm skeptical of whether access is even searching in the right
> place for the file..

I won't risk to let unrarexec be matched in any search path, user has
to specify the fullpath or relative path to mplayer's working path to
use unrar executable. Shell does the PATH search, access does not.

>
> > > > +    if (!*output || !*size || (pid == -1 && errno != ECHILD) ||
> > > > +            (pid > 0 && status)) {
> > >
> > > What's the advantage of this over a simple
> > > > if (!*output || !*size || pid == -1 || status)
> > > ?
> >
> > ECHILD maybe a valid result, because the 'MPlayer' already eaten the
> > exit status somewhere by other waitpid or sighandler.
>
> What? How is some other MPlayer code supposed to magically run in the
> middle of your code? If some signal handler did it, this is a bug that
> must be fixed, not something to be covered up by trying to treat it as
> a valid condition.

It happens on my mplayer, after check, it probably happens at line 718
of mplayer.c.

-- 
Ulion