On Tue, 2009-07-21 at 15:51 +0200, Pierre Marchand wrote:
> Vous (Pierre Marchand) avez écrit :
> > I’m getting into your recent changes and had a couple of trivial issues I
> > wan’t to know if I must fix it myself or wait for you finish the work.
> >
> > + podofo_config.h is not installed.
> > + HAVE_FONTCONFIG is publicly defined in podofo_config.h but not prefixed
> > with "PODOFO_".
> 
> Going further I see there are rather more changes than I thought at first 
> sight. 
> It’s still not clear for me if podofo_config.h is intended to be public or 
> just used at build time, I did notice it was missing when building apps 
> relying on podofo that complained PDF_INT*_TYPENAME were not naming types.

Hmm, that's a really good point, in that I've failed to separate what
should be public and what should be private (podofo compile-time only).

*headdesk*

After all the work I did getting Pdf3rdPtyForwardDecl.h etc set up to
hide PoDoFo's external header dependencies you'd think I'd not do
somthing this dumb. Sorry.

I do think moving to the config.h approach will be clearer, I just need
to split it (and the compiler compat stuff) into public/private parts
and install the public parts.

I'll see if I can look at it tomorrow.

> Regarding deprecation of pdf_long as a  ptrdiff_t, the reasons for it are 
> quite clear

I'd _really_ appreciate an explanation, then, because I'm struggling
with understanding the rationale behind how is use as a typedef for
ptrdiff_t makes sense in some places. In most of the places I see it
used, size_t would be more appropriate, and direct use of ptrdiff_t
would be clearer in most of the others.

It's also used in things like handling of the PDF xref table, where
handling of >2GB files is desirable. That'll only work on a 64-bit
machine with pdf_long as a ptrdiff_t . 

I think "pdf_long" is too generic a name and lends its self to use for
inappropriate things. If you want a string or buffer size, use size_t.
If you want an offset, use ptrdiff_t directly.

For file/stream work, I'm inclined to suggest using the STL typedefs: If
you want a file offset use std::streamoff and for a file position
std::streampos .

"pdf_long" is meaningless and generic, and IMO rather confusing as it
obscures both purpose and size/signedness.

That said, I was certainly wrong to change it in the sources as I did.
My only defense is that I was really tired and too thick to notice it.
I've reverted the alteration of the typedef. I do think pdf_long needs
to disappear though, and that it's currently used incorrectly where it
touches on file offsets, xref offsets, etc.

-- 
Craig Ringer


------------------------------------------------------------------------------
_______________________________________________
Podofo-users mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/podofo-users

Reply via email to