On Thu, Jun 19, 2008 at 02:06:54PM -0500, Tom Mueller (pkg-discuss) wrote:
> http://cr.opensolaris.org/~tmueller/cr-954/
os_windows.py:
- I'm confused why rename() and cleanup_trash() don't share the same
cache variable for the image, or the same codepath to discover it.
- Shouldn't cleanup_trash be empty_trash in Windows parlance? :)
- Couldn't the shutil.rmtree() in cleanup_trash() fail due to the files
there still being in use?
- line 144: is there a particular errno that gets set when you try to
rename on top of an existing file?
- line 158: this introduces a race. Perhaps mkdtemp() instead, and put
every file in its own directory?
- line 159: "dst" here is the source of the rename to temp, right? In
that light, does the comment make any sense? I thought you could move
a file that was in use, just not remove it (or rename on top of it or
any other file). As long as you don't lose the race, this rename
should always succeed, no?
- line 163: there's another potential race here, too. If you care, you
could recurse. Or is two tries before a traceback sufficient?
- I'd probably wrap all of the inner exception clause here in a single
function, move_to_trash() (or something like that).
t_plat.py:
- line 100: This means you're unable to perform the test, which seems
bad. Perhaps it shouldn't return silently? Maybe assert that the file
exists, or have a series of files that you try?
- line 111,126: No need for the parens around the LHS.
Danek
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
