[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kfm-devel
Subject:    Re: FW: Bug#1812: KFM memory leak: updated patch for khtmlw.
From:       Waldo Bastian <bastian () suse ! de>
Date:       1999-08-31 14:06:36
[Download RAW message or body]

On Tue, 31 Aug 1999, Lars Knoll wrote:
> But what's important,
> and there I must agree with Bjarni, is, that items might get stored twice. I
> just did a short check, where the cacheImage method is used. It doesn't
> check if the icon is already in the cache! (kbind.cpp and kfmman.cpp in kfm)

Oh wait.. I see... I was looking at:

kbind.cpp:125

    const char * pixFile = getPixmapFile( _mini );
    pixmap = pixmapCache->find( pixFile );
    if ( pixmap == 0L )
    {
        pixmap = new QPixmap;
        pixmap->load( pixFile );
        pixmapCache->insert( pixFile, pixmap );
    }                                                                           

Which is a different cache than the one in:

kbind.cpp:140
        if ( miniPixmapFile.isEmpty() )
        {
            miniPixmapFile = getIconPath( pixmapName, true );
            HTMLImage::cacheImage( miniPixmapFile.data() );
        }
        return miniPixmapFile;                                                  

> So the pCache->insert() call has to be replaced by pCache->replace().

Yes. It seems. 

That leaves the question why these images are cached in two seperate
locations.

> Also I think it's a good programming practice to set the cache to autodelete.
> One can argue if the cache clearing is really needed, but if we set the
> threshold for clearing to a high enough number (higher than in the patch), it
> won't hurt, if the cache doesn't fill up, and it helps in case it fills up. So
> where's the problem?

I wouldn't change things like this 4 days before a release. I'll have a
look at how this cache is used and if it indeed leaks as bad as the
above would suggest. To be continued.

Cheers,
Waldo

[prev in list] [next in list] [prev in thread] [next in thread]