On Thu, 2003-04-10 at 06:18, Dieter Maurer wrote:

ATT: cross post

We have a nasty FileStorage packing bug

<http://collector.zope.org/Zope/875>

The bug is caused by a strange handling of backpointers during pack's copying phase: the code checks whether the backpointer goes to a packed or unpacked position. When it goes to a packed record, it guesses what new value the backpointer should have -- and it guesses wrong. This can lead to data loss!

It actually picks an invalid location?!? I have actually been aware of this bug for a few weeks, but hadn't placed a high priority on fixing it because I didn't believe it would occur in practice :-(.

Currently, "pack" treats its two phase quite asymmetrically with respect to position resolution:

Packing phase:

backpointers are resolved "prev" and "nvprev" positions are determined via "index/nvindex"

Copying phase: backpointers are not resolved but adjusted depending on whether they point to packed or unpacked records "prev" and "nvprev" are only adjusted via "index/nvindex" when they point to packed data. Otherwise they are adjusted via a constant offset.

I do not see any reason why this should not be symmetrical:

all backpointers resolved all positions determined via "index/nvindex"

No more guessing, no longer an offset that needs to be held constant.

What is correct for packed data should be correct for not yet packed data, as well.

Do you see an error in this argument?

I don't see an error in the argument, but the logic of pack() is complicated enough that I don't have a lot of confidence. Would you care to develop a patch? There's a test -- checkPackVersionsInPast, currently disabled -- that provides at least a minimal test of the behavior. Jeremy