Count the WTFs [Python edition]

spamcourt

There's no way this code could work as documented, but even so, it manages to contain a wealth of "WHAT THE FUCK"

tmpdir = '/tmp/screenlets' + '/install-temp/'
os.system( 'mkdir %s' % tmpdir)
for d in tmpdir: #for each item in folders
	if os.path.exists(d) and os.path.isdir(d): #is it a valid folder?
		for f in os.listdir(tmpdir):
			name = f
try:
	print name
	return (name, ext)
except:
	return False

This is what happens when VB coders get into Python...

Not using os.path.join().
Using os.system().
Calling mkdir instead of using os.mkdir() or os.mkdirs().
Iterating over the tmpdir string.
Ignoring the oh-so-pertinent d variable defined in the outer loop.
Iterating through the contents of the newly-created directory.
Using a loop to get the last element of a sequence instead of just indexing.
Using an exception to detect creation of a variable.
Returning False when the other code path does not return True.

Just out of curiosity, what is this code documented to do? I need a laugh.

Wolftaur

@TehFreek said:

This is what happens when VB coders get into Python...
Not using os.path.join().
Using os.system().
Calling mkdir instead of using os.mkdir() or os.mkdirs().
Iterating over the tmpdir string.
Ignoring the oh-so-pertinent d variable defined in the outer loop.
Iterating through the contents of the newly-created directory.
Using a loop to get the last element of a sequence instead of just indexing.
Using an exception to detect creation of a variable.
Returning False when the other code path does not return True.
Just out of curiosity, what is this code documented to do? I need a laugh.

You forgot 1a: Actually needing to join two hard-coded, short paths in the first place. It's kind of like defining 'four=2+2' for future use. :)

Bulb

As written, it's a syntax error, because you can't return outside function definition. It would be nice to see the def that needs to go around it.

spamcourt

@TehFreek said:

Just out of curiosity, what is this code documented to do? I need a laugh.

@Bulb said:

As written, it's a syntax error, because you can't return outside function definition. It would be nice to see the def that needs to go around it.

def get_info_from_package_name (self, filename):
	"""Return all info we can get from the package-name or return None
	if something went wrong. If nothing failed, the returned value is
	a 4-tuple of the form: (name, version, basename, extension)."""

Here's a link to the full code in that file, for the curious

Wolftaur

@spamcourt said:

def get_info_from_package_name (self, filename):

"""Return all info we can get from the package-name or return None

if something went wrong. If nothing failed, the returned value is

a 4-tuple of the form: (name, version, basename, extension)."""

Here's a link to the full code in that file, for the curious

Oh, there's much much worse stuff in the rest of the file. For example, anything that has "the user's home directory" in the comments making /tmp/some_static_string/ is a really huge WTF. I don't care if this is some graphical application normally found on a single-user system. There is such a thing as an X-terminal. There is such a thing as people using VNC. We had a server at my last temp job that typically has about 200 people logged in via ssh, but sometimes people connect via XDMCP chooser so that they can run stuff they can't figure out how to do in a shell, or that required graphics. Now, that entire setup in and of itself was a nightmare, but...

man mkdtemp

It's there for a good reason. And it's sad that I don't even know Python but I could poke a few more holes in there.

KattMan

@TehFreek said:

This is what happens when VB coders get into Python...

This isn't a problem with VB coders, this is a problem with Wanna-be coders, they just happen to gravitate more to VB; even VB.Net has Path.Combine.

DaveK1

@TehFreek said:

Iterating through the contents of the newly-created directory.

Are you suggesting that the directory has to be empty because it's only just been mkdir'd? That would be a false inference. What about the second time the program is run?

[Also, looking at the full code, there's a snippet that was left out from the example here that extracts a tarball into the directory (whether newly-created or otherwise), but of course you couldn't have known that at the time.]

Bulb

@spamcourt said:

Here's a link to the full code in that file, for the curious

That gives whole new dimension to the WTF. A whole lot of new dimensions!

The obvious stuff about saying it returns something and than returns something completely different.
Using shell calls for lots of stuff python has native functions for—Python comes with quite comprehensive standard library including handling of .zip and .tar archives and .gz and .bz2 compression, so there should not be a single shell call in that function.
It does not seem to be cleaning up after itself, so it will just blow up when a second user calls it, because the directory will already exist, but the user will not have permissions to it. Since it is a Linux system, there is a second user.
Of course we already saw the iteration over letters of a path name—the
```
for d in tmpdir : #for each item in folders
```
where tmpdir is '/tmp/screenlets/install-temp/' will set d to '/', 't', 'm', 'p', '/', ... in turn. Funnily the loop few lines above correctly iterates over the entries in the directory.

However, not using os.path.join to combine the paths is not a WTF here—as a gnome package, it does not care about portability to Windows.

I am not sure whether I should not check whether this unsafe temporary file creation is not a reason for Release Critical bug. I would need to check the Policy.

Ilya_Ehrenburg

It's so broken and unsafe, it shouldn't be included in any self-respecting distro.