[BUG] 0.15rc2: DeprecationWarning: struct integer overflow masking is deprecated

John Arbash Meinel john at arbash-meinel.com
Sun Apr 1 02:04:51 BST 2007


Martin Pool wrote:

...

>> Now, there are 2 things we could do:
>>
>> 1) int(st.st_ino & 0xFFFFFFFF)
>> This would ignore the warning, at the potential of having a erroneous
>> cache hit. (If a files inode changed in the upper 32-bits, but the
>> mtime, ctime, size, etc did not)
>> The chance of that is pretty darn small, so I think it would be plenty
>> safe do do in the short term.
> 
> So the exposure would be that we somehow get two files whose inodes
> differ only in the top word, and which have their other fields the
> same, but different contents.  Then you replace one with the other.
> On Unix the act of switching the files would give you a new ctime, so
> (barring clocks running backwards) it still wouldn't collide.  On
> Windows ctime is (I think) the creation time, and unless renaming
> resets the mtime the switch could happen.  So it would just hinge on
> whether you are likely to ever get inodes that are the same in the
> lower 32 bits.

Well, also the files have to be exactly the same size with the same
mtime and dev number.

And in the worst case, it just means that we don't notice the file has
been changed, so we don't realize it needs to be committed. If they
'touch' the file, or add a byte, or re-save the file, etc. It will be
picked up.

I feel it is reasonable. If people are changing near-identical files in
these sorts of ways, it seems like they would have to try *really* hard
to trigger it.

> 
>> 2) Longer term, I think we are going for "%lX.%lX...." as the
>> formatting, rather than base64.encode(struct.pack('>l...')). It seems to
>> be a bit faster to generate, can handle 64 bit entries, and scales down
>> to small entries, too. I think that is the proposed change for 0.16.
>>
>> Thoughts?
> 
> So I assume you're doing the &  because we can't change the length of
> this field?  Does dirstate really care how long it is?

Well, struct.pack() cares. I think we could switch to 'Q', which is a
'long long' type. But I felt forcing it to be 64-bits for everyone, and
causing a cache miss on all existing trees for no real gain. (Though
honestly, when I first did it I didn't see 'Q'.)

Just to remind, we don't actually use any of this data for writing data
to the repository. It is only used as a hint of whether a file has
actually changed.

John
=:->




More information about the bazaar mailing list