-2

I have

Following piece of code: 

for src_filename, src_code in src_dict.iteritems(): 
try:
set.dependencies = subprocess.check_output('unifdef -s /home/c/maindir/folder/' +src_filename, shell=True)
except subprocess.CalledProcessError, e:
       print "code is bad" +set.property
       set.bad = 1
       raise
set.dependencies = list(set(set.dependencies.splitlines()))

I wanted to un-hardcode the path so I wrote following piece of code: 

filepath = os.path.join(maindirpath, "folder/")

maindir is argument here: /home/c/maindir 

 path = open(filepath)
 set.dependencies = subprocess.check_output("unifdef" '-s' path +src_filename, shell=True)

It throws following error:

TypeError: cannot concatenate 'str' and 'file' objects

I am new to python. can anybody help, where I am getting wrong?

geek_xed
  • 165
  • 1
  • 8
  • 15
  • 2
    Clearly, `src_filename` is a file object, not a filename, despite its name misleadingly claiming otherwise. – Charles Duffy Jul 09 '15 at 23:03
  • Also, `shell=True` is evil, and your code has security bugs because of it (see what happens if `src_filename` is `"hello $(rm -rf .).txt"`. – Charles Duffy Jul 09 '15 at 23:03
  • 2
    It's noteworthy, by the way, that you show how you're getting `filepath` and `path`, but not `src_filename`, which is the argument you *actually use*. Please, in the future, check that your code can be copied-and-pasted to reproduce the problem that you're asking about with no prior setup needed. See also: http://sscce.org/ – Charles Duffy Jul 09 '15 at 23:05
  • basically I want to keep the code same, just instead of hardcode path wants to use the 'path'. – geek_xed Jul 09 '15 at 23:05
  • You have invalid syntax in the third code block (`"unifdef" '-s' path +src_filename`). Please post the real code, and the traceback given with the error. – interjay Jul 09 '15 at 23:06
  • 1
    `path` is a file. To get the filename use `path.name`, – Peter Wood Jul 09 '15 at 23:08
  • Ahh! This update clarifies things a bit. It's still not a SSCCE, since other people can't copy/paste/run it (unless they have `src_dict` defined), but it makes it clear that a big misleading issue is bad choice of variable names: `src_filename` isn't a filename, it's a file, so you should probably name it `src_file`. – Charles Duffy Jul 09 '15 at 23:19
  • yes that's true. its a file. my issue is only that I don't want to enter that hardcoded path each time. I am taking maindir from user, it should join with folder and make use of it. But since i am new to python, my every try is failing. – geek_xed Jul 09 '15 at 23:21
  • Are you trying to open a directory with `path = open(filepath)`? – sancho.s ReinstateMonicaCellio Jul 09 '15 at 23:23

3 Answers3

1
for src_file, src_code in src_dict.iteritems():

  # assuming, here, that you want to honor the handle's path if already given
  filename = src_file.name
  if not '/' in filename:
    filename = os.path.join(filepath, filename)

  try:
    set.dependencies = subprocess.check_output(['unifdef', '-s', filename])
  except subprocess.CalledProcessError:
    pass # etc.

By the way, set is a bad variable name, since set is also a Python data type; you're making the data type unusable in your code by shadowing it with a like-named variable. Don't do that!

Charles Duffy
  • 280,126
  • 43
  • 390
  • 441
  • You have a file object. The file that the object is backed by already includes a path in its name, unless you're trying to refer to a like-named file in a different directory. – Charles Duffy Jul 09 '15 at 23:22
  • In the "refer to a like-named file in a different directory" case, use the `basename()` approach suggested by sancho.s -- but, ideally, couple it with the subprocess usage pattern given here, where you **don't** use `shell=True`. – Charles Duffy Jul 09 '15 at 23:27
  • *shrug*. I can't reproduce your bug, so that report does nobody any good. If your code were a proper SSCCE (particularly, **self-contained**), we could test our implementations and know what they did or didn't do, but your question here isn't usefully described enough to let anyone test their answers. – Charles Duffy Jul 09 '15 at 23:28
0

You are trying to concatenate 'unifdef -s /home/c/maindir/folder/' with src_filename The first is a string, so the second is likely a file object.

As per your edited version, you probably need

bytestr = subprocess.check_output( [ 'unifdef', '-s', filepath + os.path.basename(src_filename.name) ] )

instead of

set.dependencies = subprocess.check_output("unifdef" '-s' path +src_filename, shell=True)

(remember that subprocess.check_output returns a byte string). Then assign bytestr to whatever you need.

You might even want

try :
    bytestr = subprocess.check_output( [ 'unifdef', '-s', filepath + os.path.basename(src_filename.name) ], stderr=subprocess.STDOUT )
except subprocess.CalledProcessError :
    ...

This may work, although it is quite rudimentary, and you probably have much better options workable with the rest of your code (which we do not know).

See How do you use subprocess.check_output() in Python?

Community
  • 1
  • 1
sancho.s ReinstateMonicaCellio
  • 14,708
  • 20
  • 93
  • 185
  • That is true. How can I rearrange code so that instead of hardcode path. it will take path and concatenate it with src_filename – geek_xed Jul 09 '15 at 23:07
  • @geek_xed, how can we tell you how to rearrange code you aren't giving us in the first place? Your question needs to include code that *works to reproduce the problem*, per the rules at http://stackoverflow.com/help/mcve – Charles Duffy Jul 09 '15 at 23:08
  • The code is very big. I can't copy paste. I am getting error at that statement only. I am not sure how to make use of path. – geek_xed Jul 09 '15 at 23:09
  • @geek_xed, the MCVE and SSCCE links I've given you show you how to break down a big piece of code to a minimal reproducer. It's work, but then, you're asking us to do work for you; it's only fair that you do some work too. :) – Charles Duffy Jul 09 '15 at 23:11
  • @sancho.s, better, but do we really want to be encouraging someone to pass `src_filename.name` to a shell, when we don't know that that name will parse back to itself? Think about what happens if the name contains spaces. It'd be less buggy to use `pipes.quote` or its Python 3 `shlex.quote` replacement, but even better to modify the code to not use `shell=True`. – Charles Duffy Jul 09 '15 at 23:14
  • @CharlesDuffy - You are absolutely right. The code posted is only intended to get the OP going, not to be fail safe. – sancho.s ReinstateMonicaCellio Jul 09 '15 at 23:16
  • @sancho.s, yes, but if we "get someone going" by showing them a practice that has subtle (hard-to-catch) bugs, when will they ever learn the right way? – Charles Duffy Jul 09 '15 at 23:18
  • @CharlesDuffy - They might, or they might not. This is pointing out the specific error the OP made. All aspects are worth commenting on (the specific source of the `TypeError`, use of `subprocess.check_output`) – sancho.s ReinstateMonicaCellio Jul 09 '15 at 23:24
0

I think I understand, here is what I would try:

import os

D = os.getcwd()           # returns a string
newD = (D + '/folder/')   # concatenates the two strings together

I use this all the time, if os.getcwd() (get current working directory) isn't exactly what you are looking for, there are similar options under the os module that will also return a string. As I am sure you know, typing help(os) in IDLE should show you all the options.

THR33
  • 44
  • 3