Re[2]: [PATCH] ext.c, regex_command ()

[Pavel Roskin <proski gnu org>]

Hello!

I feel bad that I broke the code ask asked your for help and then fixed it
myself.  I hope you didn't lose much time on it.

> PR> By the way, if I hacked this function, I would first try to split
> PR> processing of type/ into a separate function.  regex_command() is too
> PR> large.  It even has two different variables "p".
> 
> Yes I though about this also but thought it wold be too intrusive. So
> I modified the code so its much more readable and contains less nasty
> constructs.

Reorganization of the code, if done by clean hands, is not the most
intrusive change.  It doesn't matter if you move everything around or not,
it matters if you don't lose anything while doing it.  Comments and
documentation are also important.

> So lets say you're missing 'file' command. No problem - the original
> code would try to check 'content_string' each time a 'type/' is found
> in mc.ext though it is filled with garbage.

Fixed.

> >> 1. Why this piece of code doesn't use the subshell ?
> 
> PR> Why should it?  Especially when we are feeding data from a remote server 
> PR> into "file", bad things can happen if "file" dies.
> 
> I don't know what kind of things you refer but I agree with you though
> :) And I'm sure you have a reason. Still it could be good to have a
> single interface for executing programs. Just a thought - no need to
> have a long discussion on that - I don't know all the MC code :)

I mean an attempt to kill "file" from a compromized FTP server by a
carefully constucted garbage and run some commands with your rights, e.g.  
to get your ssh key or your e-mail.  Maybe I'm too paranoid.

The subshell is very fragile.  It lives its own life.  mc thinks that it 
knows what the shell does, but it's not always true.  The subshell can be 
"busy", i.e. it got some keystrokes, but didn't return the prompt.

I would rather have one more way to execute commands - separate 
interactive process - for things like running external spellchecker in the 
editor.

> PR> I don't think it should be redirected to stdout - it should be ignored of 
> PR> put to a separate file and then displayed.  ext.c is a single caller of 
> PR> mc_doublepopen() now, so it should be easy.
> 
> Ok I chose the wrong redirection  :)  I was thinking /dev/null but I
> dont know if all platforms have /dev/null.

All platforms we support either have /dev/null or don't have
mc_doublepopen().  The later is the Win32 native port, that never quite 
worked since 4.5.18 or so.  I'm almost certain that I'll kill it earlier 
or later - nobody really wants to fix it.

> Well we better ignore it because this way we have to additional code to
> check that file and parse the output - I think it is not worth. But it's
> not good to have it in MC prompt.

Of course it's not good.

-- 
Regards,
Pavel Roskin