Re: [REVIEW-3.5] crasher fdo#45987

On 18/05/12 14:49, Markus Mohrhard wrote:
> Hey Noel
>
> 2012/5/18 Kohei Yoshida<kohei.yoshida@gmail.com>:
> > On Fri, May 18, 2012 at 9:14 AM, Noel Power<nopower@suse.com>  wrote:
> >
> > > > http://cgit.freedesktop.org/libreoffice/core/commit/?id=8b1d29bc9b00bc2730738a990023a65ab6e0219b
> > > > &
> > > >
> > > > http://cgit.freedesktop.org/libreoffice/core/commit/?id=abb26f51eea0399754cc8f5b7d7a7d648d68f630
> > >
> > > I took it that it should work how I outlined above and committed a further
> > > fix which should safeguard against illegal access, please additionally
> > > consider
> > > http://cgit.freedesktop.org/libreoffice/core/commit/?id=8352eb5a1af1eb44550a9d60d31e6c2fb2dc43b9
> > So, these extra range checks should be safe; however....  The original
> > intention of *not* checking the bound of the tab index was that we
> > assumed that this list would be in sync with the maTab in ScDocument
> > at all times.  So, if they are out of sync then something else may be
> > causing this problem...
> >
> > Let me ping Markus here.  He worked on the rework of ScTable storage,
> > and this issue is related to that work.
> I agree with Kohei. Needing a range check at this place will most
> likely hide a underlying problem. The table container in ScViewData
> and the one in ScDocument must always be in sync. I fear that this
> might have been a problem that has always been there.
>
> Loosing the sync between the two data structures will result in wrong
> sheet numbers coming from the view part. Let me check this bug report
> as soon as I have a clean tree.
ok I might have jumped the gun ( should have waited for the answer ;-) 
), fair enough, feel free to revert those commits in master then. 
Probably the ViewData should be initialised with the same number of Tabs 
as the document, being the view doesn't it just track what views have 
been selected ? ( so probably it has just 1 setup initially e.g. the 
current view ) saying that though ( now that I think of it I could swear 
that in fact there were the size of the maTabData was 2 in the bug 
scenario which is strange, maybe some over zealous allocation ? )

Noel