KProcess overhaul [PATCH]

[Ralf Habacker]

Oswald Buddenhagen schrieb:
> On Mon, Apr 17, 2006 at 01:57:25PM +0200, Ralf Habacker wrote:
>   
>> 8-10 not implemented (on windows not required)
>>
>>     
> this is simply not true. only pty support is not needed; redirections in
> general *are* needed, now even more than before, as we want portability,
> so we can't simply use the unix shell syntax anymore.
>   
Not sure, what you mean exactly here and where this support is used or 
required in the kde sources. Can you give an example ?
>   
>> new use cases
>> 11. run a process using a command line shell
>>
>>     
> it should be noted that this bears *considerable* portability problems.
>   
yes, which will still exists in some area, where the unix command have 
other names as on windows.
> the quoting rules are *totally* different 
How are they different ?
> and cmd.exe is slightly limited in its possibilities. 
>   
yes, this limits the available commands on windows.
> the only thing the shell should be used for are user-supplied commands.
> for internal stuff, either the code should be rewritten not to use a
> shell (as stated before, the majority of the cases is a poor man's
> workaround for the lack of redirection support in kprocess), or, if it
> is not hard-coded (like in .desktop files), it should be written in unix
> shell syntax and interpreted internally - i intend to put a *simple*
> shell execution semantics interpreter into KShell.
>   
Agreed, but KProcess need an interface for executing a shell. It is okay 
as it is ?
Additional, do you have an example for this ?

One example I remember is kdecore/Ktimzeones.cpp with the following unix 
shell command

    KProcess::executeShell("/bin/grep" QStringList() << "-h" << "^Zone" 
<< m_zoneinfoDir << "/src/*" << temp.name() << "|" <<
     "/bin/awk" << "'{print \"??\\t+9999+99999\\t\" $2}'");

which will never be used on windows, because there are no timezone 
files. So no problem here.

>> Any comments or objectivities ?
>>
>>     
> it's objections. ;)
> yes, i have some.
>
> 1) on unix, -c is required (where it is /c on windoze). remember that
> everything in windows that is not fundametally new (is there something
> like that at all?) is based on unix concepts. :-P
> btw, you botched the if-else cascade. 
which comes from the old KProcess code. If other shells beside /bin/sh 
aren't required, this code could be reduced very much.

> but there's no reason for
> shellOptions anyway, as it is required to be consistent among shells.
>   
QProcess::start()'s or QProcess:execute()'s first parameter must be an 
executable without any parameters, otherwise it couldn't be located in 
the path variable.
This require to save parameters elsewhere in this case shellOptions. Is 
this okay for you ?
> 2) you botched the command composition. it is 
> "shell -c <all-arguments-concatenated>", not "shell -c <arg> <arg> ...".
> on windows that does not matter, as everything is more or less simply
> concatenated into one string anyway, but on unix it does.
>   
I don't know how to implement this on unix. Could you update the patch 
with the related fixes.

> 3) you leak the d pointer
fixed by adding a destructor.

Ralf


-------------- next part --------------
A non-text attachment was scrubbed...
Name: kprocess-0.4.zip
Type: application/x-zip-compressed
Size: 3875 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20060417/b608e991/attachment.bin>