Count the WTFs [Python edition]



  • 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...

    1. Not using os.path.join().
    2. Using os.system().
    3. Calling mkdir instead of using os.mkdir() or os.mkdirs().
    4. Iterating over the tmpdir string.
    5. Ignoring the oh-so-pertinent d variable defined in the outer loop.
    6. Iterating through the contents of the newly-created directory.
    7. Using a loop to get the last element of a sequence instead of just indexing.
    8. Using an exception to detect creation of a variable.
    9. 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.



  • @TehFreek said:

    This is what happens when VB coders get into Python...
    1. Not using os.path.join().
    2. Using os.system().
    3. Calling mkdir instead of using os.mkdir() or os.mkdirs().
    4. Iterating over the tmpdir string.
    5. Ignoring the oh-so-pertinent d variable defined in the outer loop.
    6. Iterating through the contents of the newly-created directory.
    7. Using a loop to get the last element of a sequence instead of just indexing.
    8. Using an exception to detect creation of a variable.
    9. 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. :)



  • 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.



  • @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



  • @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.



  • @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.



  • @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.]



  • @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.



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


Log in to reply
 

Looks like your connection to What the Daily WTF? was lost, please wait while we try to reconnect.