On Jul 19, 2011, at 12:51 AM, Matúš Kukan wrote: > Hi Joe, > > On 19 July 2011 06:40, Joseph Powers <jpower...@cox.net> wrote: >> I'd like someone doing a Unix build to review this for me. I compile Mac and >> this is Unix only code so I don't just want to push and hope... >> > First I thought it would compile and want just to write something but > then I tried and it doesn't. > But my question is: > Would not it be better to replace List with std::list ? Or if vector I > don't like erase because it's not effective. > In this case I'd use maBmpList.pop_back(). On the first sight I > thought you have mistake in:
A List<> would be better; however, it's a list of pointers so the size isn't
that big.
> void ImplSalBitmapCache::ImplRemove( X11SalBitmap* pBmp )
> {
> - for( ImplBmpObj* pObj = (ImplBmpObj*) maBmpList.Last(); pObj;
> pObj = (ImplBmpObj*) maBmpList.Prev() )
> + for( size_t i = maBmpList.size(); i; )
> {
> + ImplBmpObj* pObj = maBmpList[ --i ];
> if( pObj->mpBmp == pBmp )
> {
> - maBmpList.Remove( pObj );
> + maBmpList.erase( maBmpList.begin() + i );
> pObj->mpBmp->ImplRemovedFromCache();
> mnTotalSize -= pObj->mnMemSize;
> delete pObj;
>
> But then I realized you are decreasing i in ImplBmpObj* pObj = maBmpList[ --i
> ];
> So - maBmpList.erase( maBmpList.begin() + i ); is in fact pop_back and
> it's effective but personally I'd use the latter to avoid mistakes.
It's a loop and it's not just removing the last entry. It's only removing the
entry that matches the one passed. (I don't know why we're starting at the end
since the same pointer shouldn't be in the list twice; however, if the same
pointer gets in the list twice, then we'll always remove the last one instead
of the first one and I didn't wont to change this behavior).
> Now here is what I got on 32bit Ubuntu:
>
> vcl/unx/generic/gdi/salbmp.cxx: In member function ‘void
> ImplSalBitmapCache::ImplAdd(X11SalBitmap*, sal_uLong, sal_uLong)’:
> vcl/unx/generic/gdi/salbmp.cxx:1218: error: invalid use of incomplete
> type ‘struct ImplSalBitmapCache::ImplBmpObj’
> vcl/inc/unx/salbmp.h:253: error: forward declaration of ‘struct
> ImplSalBitmapCache::ImplBmpObj’
Ok, just guessing but:
+struct ImplBmpObj;
+
class ImplSalBitmapCache
{
private:
+ typedef ::std::vector< ImplBmpObj* > BmpList_impl;
- List maBmpList;
+ BmpList_impl maBmpList;
sal_uIntPtr mnTotalSize;
Would most likely work better. I was defining "struct ImplBmpObj" inside the
ImplSalBitmapCache class and in the .cxx the actual struct was defined outside
the class; thus, we never defined the expected structure.
> I was not investigating where the problem is, I think you can handle it.
Can you try the new version of the patch?
> All the best,
> Matus
Thanks for helping,
Joe P.
0001-Replace-List-with-std-vector-ImplBmpObj.patch
Description: Binary data
_______________________________________________ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
