[PATCH 05 of 11] cmdutil: securely rename a to A on disk

[Simon Heimberg]

Am Freitag, den 07.08.2009, 23:20 +0200 schrieb Mads Kiilerich: 
> Simon Heimberg wrote, On 08/07/2009 09:43 PM:
> > # HG changeset patch
> > # User Simon Heimberg<simohe at besonet.ch>
> > # Date 1249656995 -7200
> > # Node ID 50e9f6b4d8878fd867a250c5be2578330b7f4769
> > # Parent  76406c3720caa61cf9d466b43d2aac68b15ec362
> > cmdutil: securely rename a to A on disk
> >

[snip] 

> >
> > +def temprename(src):
> > +    """renames a file to a temporary name
> > +       the new name is returned"""
> > +    for tries in xrange(10):
> > +        temp = '%s-%08x' % (src, random.randint(0, 0xffffffff))
> > +        try:
> > +            os.rename(src, temp)
> > +            return temp
> > +        except IOError:
> > +            continue
> > +    raise IOError, (errno.EEXIST, "No usable temporary filename found")
> > +
> >    
> 
> Does moving a function count as touching it?
In this case definitively yes, I have already changed some things. 
> 
> IF you are touching it anyway then it it would be more secure with an 
> extra "if os.path.exists(temp): continue".
You are right. I thought os.rename would raise an error when the
destination file exists. So I removed os.path.exists, but that is wrong.
The file could be created after os.path.exists is called and before
os.rename. Is this problematic? Should the file be created with
tempfile.mkstemp and overwritten with os.rename?
> 
> This function can also give strange errors on filenames close to the max 
> length. Perhaps we could drop the basename and use a short "random" name 
> in dirname...
Does this mean the length of the filename or the entire path? If it is
the first, temprename could look like this:

def temprename(src):
    """renames a file to a temporary name
       the new name is returned"""
    temp = tempname.mktemp(suffix='tmpmv', dir=os.path.dirname(src))
    os.rename(src, temp)
    return temp

Is there a reason why tempname.mktemp was not used?

Are there other concerns to the patch series? Shall I resend the entire
series or only this patch?

The other patches of this series do not depend on this patch (only
test-casefolding is broken on linux with hfs). 

[snip]