Re: [Evolution-hackers] New patches for Inline PGP support

[Not Zed <notzed ximian com>]

Hi Matt,

I think i've spotted the memory problem, you're unreffing the return
from camel_medium_get_content_object in the code above the problem one.
This shouldn't be freed (i guess it should be a const, but it isn't), as
it is really just a member access.  Valgrind probably wouldn't show much
because camel objects are allocated in a pool allocator so it would look
like good memory.

Try that?  Just looking at the patches i'd say they're pretty well ready
otherwise (not that i've built or tested them ;-), if that fixes the
problem.

Sorry for taking so long, i thought this was going to be a big job, but
i dont really have anything more to add!

Cheers,
 Michael

On Thu, 2005-06-16 at 00:24 +1200, Matt Brown wrote:
> On Thu, 2005-06-02 at 12:11 +0800, Not Zed wrote:
> > Looking good matt,
> > 
> > I have a few comments, naturally :)
> 
> Of course, much appreciated :)
> <snip>
> > Anyway, I think it is just a matter of a little re-arranging of the
> > code, most of the parts look good otherwise :)  Oh, remember to clean up
> > properly in the failure cases, i.e. the if (!valid) code doesn't do any.
> 
> OK. Your suggestions all seemed blindingly obvious in hindsight. So
> there are some new patches linked below that hopefully tidy all this
> up. 
> 
> > I'm not sure about the text/plain formatting changes; I see why you're
> > doing it, but it will affect the formatting, and add weird extra boxes
> > in certain cases I think.  I guess i'll have to see how it looks.
> 
> I've broken this change out into a separate patch to ease testing.
> 
> One tiny new problem that has been introduced with the rearrangement of
> code and has me beat. You'll notice at the bottom of the
> efh_inlinepgp_signed function the call to free the generated mime part
> (opart) is currently commented out. This is bad, however uncommenting it
> leads to all sorts of nasty assertions and references to invalid objects
> when trying to view a message that has nested inline pgp signed content.
> If I leave the part unfreed then these errors all dissapear at the
> expense of a memory leak. 
> 
> I'm absolutely stumped as to the cause of this, and I can't get valgrind
> to give me anything useful, so I'm hoping that somebody will spot
> something blindingly obvious I'm doing wrong and correct it!
> 
> Patches are at
> http://www.mattb.net.nz/patches/evolution/eds-inline-pgp-v2.patch
> http://www.mattb.net.nz/patches/evolution/evo-inline-pgp-v2.patch
> http://www.mattb.net.nz/patches/evolution/evo-inline-pgp-formatchanges.patch
> 
> If it wasn't for the above bug I think I would be ready to start sending
> them to -patches.
> 
> Regards
>