Re: [Pdfedit-cvs] pdfedit/src/kernel pdfoperators.cc, 1.50, 1.51 pdfoperators.h, 1.70, 1.71 stateup

SourceForge Headquarters 225 Broadway Suite 1600 San Diego, CA 92101 +1 (858) 454-5900

On Mon, Nov 10, 2008 at 10:07:24AM +0000, Jozef Misutka wrote:
> 
> 
> 
> > Date: Mon, 10 Nov 2008 10:41:26 +0100
> > From: ms...@gm...
> > To: mis...@us...
> > Subject: Re: [Pdfedit-cvs] pdfedit/src/kernel pdfoperators.cc, 1.50, 1.51	pdfoperators.h, 1.70, 1.71 stateupdater.cc, 1.26, 1.27
> > CC: pdf...@li...
> > 
> > On Mon, Nov 10, 2008 at 09:34:39AM +0000, Jozef Misutka wrote:
> > > Update of /cvsroot/pdfedit/pdfedit/src/kernel
> > > In directory ddv4jf1.ch3.sourceforge.com:/tmp/cvs-serv18250
> > > 
> > > Modified Files:
> > > 	pdfoperators.cc pdfoperators.h stateupdater.cc 
> > > Log Message:
> > > 
> > > 
> > > -- fixed bugs in FontData (reimplemented) 
> > 
> > Which bugs - mind to describe?
> 
> crash on one testfile:
> 
> fontName = strdup(font->getName()->getCString());
> 
> font->getName()->getCString() was not initialized

OK? I though that this could never happen and it really didn't in my
testing (which was obviously not sufficient - as well as review...).
Why didn't you just add checks and you did refactore whole class?
I understand that std::string is much simpler on using but this should
be as much light as possible because it is a part of _each_ text
operator! Is the overhead worth the simplicity?

> 
> > 
> > > and taking over (any objections michal?)
> > 
> > No problem
> > 
> > > -- removed mutable as it is not needed
> > > 
> > > Index: stateupdater.cc
> > > ===================================================================
> > > RCS file: /cvsroot/pdfedit/pdfedit/src/kernel/stateupdater.cc,v
> > > retrieving revision 1.26
> > > retrieving revision 1.27
> > > diff -u -d -r1.26 -r1.27
> > > --- stateupdater.cc	29 Oct 2008 15:10:51 -0000	1.26
> > > +++ stateupdater.cc	10 Nov 2008 09:34:37 -0000	1.27
> > > @@ -440,7 +440,7 @@
> > >  
> > >  		// This can happen in really damaged pdfs
> > >   		if (state->getFont()) {
> > > -			const TextSimpleOperator *txtOp = dynamic_cast<const TextSimpleOperator*>(op.get());
> > > +			TextSimpleOperator *txtOp = dynamic_cast<TextSimpleOperator*>(op.get());
> > >  			assert(txtOp);
> > >  			txtOp->setFontData(state->getFont());
> > >  			std::string rawStr;
> > > @@ -505,7 +505,7 @@
> > >  		state->textMoveTo(tx, ty);
> > >  
> > >  
> > > -		const TextSimpleOperator *txtOp = dynamic_cast<const TextSimpleOperator*>(op.get());
> > > +		TextSimpleOperator *txtOp = dynamic_cast<TextSimpleOperator*>(op.get());
> > >  		assert(txtOp);
> > >  		txtOp->setFontData(state->getFont());
> > >  		std::string rawStr;
> > > @@ -582,7 +582,7 @@
> > >  			rc->yright = max( rc->yright, max( h_rc.yleft, h_rc.yright ) );
> > >  		}// for
> > >  		
> > > -		const TextSimpleOperator *txtOp = dynamic_cast<const TextSimpleOperator*>(op.get());
> > > +		TextSimpleOperator *txtOp = dynamic_cast<TextSimpleOperator*>(op.get());
> > >  		assert(txtOp);
> > >  		txtOp->setFontData(state->getFont());
> > >  		// return changed state
> > > 
> > > Index: pdfoperators.cc
> > > ===================================================================
> > > RCS file: /cvsroot/pdfedit/pdfedit/src/kernel/pdfoperators.cc,v
> > > retrieving revision 1.50
> > > retrieving revision 1.51
> > > diff -u -d -r1.50 -r1.51
> > > --- pdfoperators.cc	29 Oct 2008 15:10:51 -0000	1.50
> > > +++ pdfoperators.cc	10 Nov 2008 09:34:37 -0000	1.51
> > > @@ -237,30 +237,25 @@
> > >   */
> > >  class TextSimpleOperator::FontData 
> > >  {
> > > -	char * fontName;
> > > -	char * fontTag;
> > > +	string fontName;
> > > +	string fontTag;
> > >  public:
> > >  	FontData(GfxFont* font)
> > >  	{
> > > -		fontName = strdup(font->getName()->getCString());
> > > -		fontTag = strdup(font->getTag()->getCString());
> > > -	}
> > > -	~FontData()
> > > -	{
> > > -		if(fontName)
> > > -			free(fontName);
> > > -		if(fontTag)
> > > -			free(fontTag);
> > > +		if (font->getName() && font->getName()->getCString())
> > > +			fontName = strdup(font->getName()->getCString());
> > > +		if (font->getTag() && font->getTag()->getCString())
> > > +			fontTag = strdup(font->getTag()->getCString());
> > >  	}
> > >  
> > >  	const char * getFontName()const
> > >  	{
> > > -		return fontName;
> > > +		return fontName.c_str();
> > >  	}
> > >  
> > >  	const char * getFontTag()const
> > >  	{
> > > -		return fontTag;
> > > +		return fontTag.c_str();
> > >  	}
> > >  };
> > >  
> > > @@ -317,7 +312,7 @@
> > >  	return fontData->getFontName();
> > >  }
> > >  
> > > -void TextSimpleOperator::setFontData(GfxFont* gfxFont)const
> > > +void TextSimpleOperator::setFontData(GfxFont* gfxFont)
> > >  {
> > >  	assert(gfxFont);
> > >  	if (!gfxFont)
> > > 
> > > Index: pdfoperators.h
> > > ===================================================================
> > > RCS file: /cvsroot/pdfedit/pdfedit/src/kernel/pdfoperators.h,v
> > > retrieving revision 1.70
> > > retrieving revision 1.71
> > > diff -u -d -r1.70 -r1.71
> > > --- pdfoperators.h	29 Oct 2008 15:10:51 -0000	1.70
> > > +++ pdfoperators.h	10 Nov 2008 09:34:37 -0000	1.71
> > > @@ -128,11 +128,11 @@
> > >  class TextSimpleOperator: public SimpleGenericOperator
> > >  {
> > >  	// forward declaration
> > > -	struct FontData;
> > > +	class FontData;
> > >  
> > >  	/** Font data for later identification of associated font
> > >  	 */
> > > -	mutable FontData* fontData;
> > > +	FontData* fontData;
> > >  protected:
> > >  	/** Finds current font for operator from fontName.
> > >  	 * Uses resources from content stream to retriev font by name.
> > > @@ -167,7 +167,7 @@
> > >  	 * This method doesn't influence operator itself (or its operands).
> > >  	 * @param gfxFont Xpdf GfxFont instance.
> > >  	 */
> > > -	void setFontData(GfxFont* gfxFont)const;
> > > +	void setFontData(GfxFont* gfxFont);
> > >  
> > >  	/** Returns font name for this operator.
> > >  	 * May return null if setFontData hasn't been called yet.
> > > 
> > > 
> > > -------------------------------------------------------------------------
> > > This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
> > > Build the coolest Linux based applications with Moblin SDK & win great prizes
> > > Grand prize is a trip for two to an Open Source event anywhere in the world
> > > http://moblin-contest.org/redirect.php?banner_id=100&url=/
> > > _______________________________________________
> > > Pdfedit-cvs mailing list
> > > Pdf...@li...
> > > https://lists.sourceforge.net/lists/listinfo/pdfedit-cvs
> > 
> > -- 
> > Michal Hocko
> > 
> > -------------------------------------------------------------------------
> > This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
> > Build the coolest Linux based applications with Moblin SDK & win great prizes
> > Grand prize is a trip for two to an Open Source event anywhere in the world
> > http://moblin-contest.org/redirect.php?banner_id=100&url=/
> > _______________________________________________
> > Pdfedit-devel mailing list
> > Pdf...@li...
> > https://lists.sourceforge.net/lists/listinfo/pdfedit-devel
> 
> _________________________________________________________________
> See how Windows? connects the people, information, and fun that are part of your life
> http://clk.atdmt.com/MRT/go/119463819/direct/01/
-- 
Michal Hocko