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

[Rich Felker]

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..

> 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..

> > > +    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.

Rich