[GRASS-dev] Re: [GRASSGUI] Re: [grass-addons] r659 -
michael.barton at asu.edu
Thu May 17 17:15:54 EDT 2007
This is helpful. But I will wait to make any changes until this is worked
out--since it looks like we will also need to redefine the original command
string (i.e., to make it a proper list) prior to calling cmd.Command.
On 5/17/07 8:56 AM, "Glynn Clements" <glynn at gclements.plus.com> wrote:
> Martin Landa wrote:
>> I am not quite sure if this patch OK...
>> - self.module = subprocess.Popen(self.cmd, shell=True,
>> + cmdList = cmd.split(' ')
>> + self.module = subprocess.Popen(args=cmdList, shell=False,
> Nope. In fact, it's a good example of why the interface is wrong
> (think about how it will behave if you have a space in an argument).
> The problem isn't the implementation, it's the interface.
> Let me put it another way: which of the following is correct?
> 1. int main(char *cmd)
> 2. int main(int argc, char **argv)
> The correct answer is #2. A command isn't a string, it's a list of
> strings, and that is the interface which you need to provide.
> Taking a list of strings, combining them to form a single string, then
> splitting the list into multiple strings will introduce errors, i.e.
> the list you end up with won't always be the original list.
> The existing interface splits them according to the shell's syntax,
> which has several problems; not least of which is that /bin/sh and
> cmd.exe have radically different syntax, meaning that you would often
> need to write separate Unix/Windows versions of any code which calls
> You're proposed patch is worse, in that it becomes entirely impossible
> to pass an argument which contains a space (these are quite common for
> filenames on Windows).
> The only realistic solution is not to combine them in the first place.
> IOW, the cmd parameter should be a list, and Popen should be called
> self.module = subprocess.Popen(args=self.cmd,
> You then need to change everything which calls cmd.Command() to pass a
> list, not a string.
Michael Barton, Professor of Anthropology
School of Human Evolution & Social Change
Center for Social Dynamics & Complexity
Arizona State University
More information about the grass-dev