5

I know that it is bad practice to use shell=True for subprocesses. However for this line of code, I'm not sure how to execute it with shell=False

subprocess.Popen('candump -tA can0 can1 >> %s' %(file_name), shell=True)

Where the command I want to run is:

candump -tA can0 can1 >> file_name

Where file_name is /path/to/file.log

avelampudi
  • 316
  • 2
  • 6
  • 16
  • 2
    BTW: note that `'%s' % (filename)` is exactly the same as `'%s' % filename`. `()` do **not** create a tuple, the `,` does! So if you want to create a 1-element tuple do `(filename, )`. – Bakuriu Jan 04 '17 at 19:09

2 Answers2

8

You can't directly use piping in the command the way you do with shell=True, but it's easy to adapt:

with open(file_name, 'ab') as outf:
    proc = subprocess.Popen(['candump', '-tA', 'can0', 'can1'], stdout=outf)

That opens the file at the Python level for binary append, and passes it as the stdout for the subprocess.

ShadowRanger
  • 143,180
  • 12
  • 188
  • 271
0

Only shell mode supports inline piping operators so you'll need to do the redirection manually. Also, you'll need to split your command line into individual arguments which you can either do manually or have the shlex module do for you:

subprocess.Popen(shlex.split('candump -tA can0 can1'), stdout=open(file_name, 'ab'))
kanaka
  • 70,845
  • 23
  • 144
  • 140
  • Why would you advise `shlex.split()`, rather than an explicit argv array/list? – Charles Duffy Jan 04 '17 at 18:51
  • @CharlesDuffy using shlex keeps it closest to the original formulation of the problem. Manual splitting is fine though. I clarified the answer a bit. – kanaka Jan 04 '17 at 18:53
  • *nod*. I consider the list approach in keeping with ["explicit is better than implicit"](https://www.python.org/dev/peps/pep-0020/#id3), but opinions certainly do vary on matters of style. – Charles Duffy Jan 04 '17 at 18:55
  • 1
    ...OTOH, I think there's a very strong argument to be made that the explicit list approach results in fewer bugs: Someone using `shlex.split('candump -o %s' % file)` is prone to issues (ie. with filenames containing spaces) that someone using `['candump', '-o', file]` isn't. – Charles Duffy Jan 04 '17 at 18:57
  • @CharlesDuffy yes, that's a good point. Shlex should not be used with unsanitized/unescaped input. I generally only use it with literals/unconstructed commands because it's easier to see the command line without the commas and quotes in the middle (especially if the command line contains quotes and/or commas). – kanaka Jan 04 '17 at 19:09
  • From where I stand, distinguishing between syntactic and literal quotes is critical, and folks reading and writing Python probably know Python syntax better than they know shell syntax, so it's easier for a reader to correctly grok what is and isn't literal if it's using only their native language's escaping facilities rather than one kind of quoting layered under another. But, yes, matters-of-style, opinions-differ, &c. – Charles Duffy Jan 04 '17 at 19:17
  • (There *are* certainly nonintuitive aspects to POSIX shell quoting -- for instance, the inability to use `'this is broken: \' <- not a literal quote'` to put a literal backtick inside a string in shell [as the backslash is treated as literal, not a quote character] is something I'd expect to be surprising to most folks coming from Python). – Charles Duffy Jan 04 '17 at 19:19