Am Friday 21 December 2007 schrieb Craig Ringer: > Please reply to the list, not to me, or use "reply to all" so that the > list gets a copy. That way, others (who usually know more than me) can > see the discussion and offer their input. > > Валерий Польских wrote: > > This function called from: > > > > > > > > void PdfStream::GetFilteredCopy( char** ppBuffer, long* lLen ) const > > > > > > > > *ppBuffer = stream.TakeBuffer(); > > > > } > > ... which is again passing ownership of the buffer back to the caller. > Again, it really should say that it's handing over ownership, or use a > mechanism that makes that explicit.
Yes, the caller has to free the buffer. The API documentation does state
correctly that the caller has to free the buffer. I think this is the most
useful behaviour as streams can be large and this makes sure that the data is
not copied unnecessarily around and the caller can free the buffer as soon as
possible.
>
> > which in turn called from:
> >
> >
> >
> > void PdfMemStream::Uncompress()
> >
> > {
> >
> > long lLen;
> >
> > char* pBuffer;
> >
> >
> >
> > TVecFilters vecEmpty;
> >
> >
> >
> > if( m_pParent && m_pParent->IsDictionary() &&
> > m_pParent->GetDictionary().HasKey( "Filter" ) && m_buffer.GetSize() )
> >
> > {
> >
> > this->GetFilteredCopy( &pBuffer, &lLen );
> >
> > this->Set( pBuffer, lLen, vecEmpty );
> >
> > m_pParent->GetDictionary().RemoveKey( "Filter" );
> >
> >
> >
> > // TODO: DS:
> >
> > // If there is a decode params dictionary it might
> >
> > // be necessary to remove it
> >
> > }
> >
> > }
> >
> >
> >
> > PdfStream::Set(:) also do not release buffer.
>
> You're quite right. I'd say the buffer should be freed by the caller
> after invoking PdfStream::Set(...), and the documentation clearly needs
> some updates to indicate that the preceeding two functions transfer
> ownership of the bufffer.
>
> I imagine that when the code was originally written it used to take
> ownership of the buffer. When PdfStream was split out into PdfMemStream
> and PdfFileStream and PdfMemStream was rewritten to use a
> PdfRefCountedBuffer the change was missed.
>
> Personally I'd prefer to avoid these issues by extending the use of
> PdfRefCountedBuffer through more of the code ( in place of raw malloc'd
> buffers). It's a matter of having the time to do it, and the familiarity
> with the code in question. The use of all this C-style raw memory
> allocation is highly error prone especially in software that's subject
> to ongoing change.
>
> Dom, do you tend to agree with the above fix - to free the buffer after
> calling PdfStream::Set(...) ? Or was your intent otherwise?
Yes, It has to be free'd after a call to Set. Set gets a const char* as
pointer so PoDoFo has no way to free the buffer (well it has a way to do so
as usual in C++, but we only write clean code - right? ). As a reason the
caller has to free the buffer again and the above code should be fixed.
I will commit the fix in a minute.
best regards,
Dom
--
**********************************************************************
Dominik Seichter - [EMAIL PROTECTED]
KRename - http://www.krename.net - Powerful batch renamer for KDE
KBarcode - http://www.kbarcode.net - Barcode and label printing
PoDoFo - http://podofo.sf.net - PDF generation and parsing library
SchafKopf - http://schafkopf.berlios.de - Schafkopf, a card game, for KDE
Alan - http://alan.sf.net - A Turing Machine in Java
**********************************************************************
signature.asc
Description: This is a digitally signed message part.
------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________ Podofo-users mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/podofo-users
