Hi Kohei, On Monday, 2012-01-30 16:35:46 -0500, Kohei Yoshida wrote:
> I'd like > > http://cgit.freedesktop.org/libreoffice/core/commit/?id=e2b11f4fd79dce4116badb0ecf6477546ca5d0d4 > > cherry-picked to the 3-5 branch. It is probably too late for change > like this to be in the 3-5-0 branch so I won't even try. I'd like to ask for an improvement ;) see below. > To reproduce the problem, open the following file in Calc > > http://people.freedesktop.org/~kohei/test-formula-fill-crash.ods > > then > > 1. select B1:B65536 (or just hit ctrl-shift-up) > 2. fill down (or ctrl-D if you use the default key binding). > > That will currently either crash, or do a totally wrong thing. If it > doesn't crash, try undo and redo and it will eventually crash. > > The reason is that, during the fill, the formula token instance inside > ScFormulaResult gets "copied" i.e. it re-uses the existing instance and > increases its reference counter by one. The problem is, this counter is > unsigned 16-bit integer, and as soon as it goes above 65535 it rolls > back to zero, and eventually the token instance gets deleted > prematurely. Argh, yes, right. > The above change ensures that the formula result is cleared after each > formula cell instance gets copied. We don't need to copy the formula > result during fill because they get re-interpreted once the copying is > complete. True. Though the patch fixes this only for the Fill case, the same crash happens when copying B1 to clipboard, selecting B2:B65536 and paste. That needs to be handled similar. Probably best would be to revert this commit and change the ScFormulaResult copy-ctor to create a clone of the token instead so one doesn't need to bother with it at various places. > As an aside, although it's not necessary for this fix, on master we > should probably use unsigned 32-bit integer to store reference counter > for this just to future-proof ourselves. 16-bit integer seems a bit too > small for this purpose. I'd not recommend using 32-bit for the token reference count, that would significantly increase memory footprint of every formula. Single tokens weren't meant to be shared across formula cells. Eike -- LibreOffice Calc developer. Number formatter stricken i18n transpositionizer. GnuPG key 0x293C05FD : 997A 4C60 CE41 0149 0DB3 9E96 2F1A D073 293C 05FD
pgpaWecucicuf.pgp
Description: PGP signature
_______________________________________________ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
