Hi,

You're right. I missed that completely.
The reason is that i didn't compile the code.  I just assumed that a method named "Write" should override a similar method in the base class.

Of course it's possible to rename the method like you suggested. However, I feel that this method should indeed override the base class' Write.
After looking at the code for the method, I realized that it duplicates some of the code from the base class. I believe that this should be avoided.
In addition, it seems like PdfSigIncWriter is writing all the objects of the document (I'm not 100% sure of that). This is certainly not needed.

Bottom line: I can certainly do a rename but I think that it would be better to have a bit of a different architecture.

Ilan

On 5/15/2014 00:08, zyx wrote:
On Mon, 2014-05-12 at 15:15 +0300, Ilan Zisser wrote:
I didn't see this. My revision didn't include PdfSigIncWriter.h. 
The reason for the  warning is obvious, missing "virtual" before the
method Write.
	Hi,
it's not that simple, unfortunately. The problem isn't "missing"
'virtual' qualifier (you are not required to repeat 'virtual qualifier
on methods which the base class has defined as virtual), the problem is
that the function prototype is different from the parent class' Write.
Using both patches shows only almost-the-same warning:

        In file included from podofo-trunk/src/base/PdfSigIncWriter.h:37:0,
                         from podofo-trunk/src/base/PdfSigIncWriter.cpp:34:
        podofo-trunk/src/base/PdfWriter.h:103:18: warning: ‘virtual void PoDoFo::PdfWriter::Write(PoDoFo::PdfOutputDevice*)’ was hidden [-Woverloaded-virtual]
             virtual void Write( PdfOutputDevice* pDevice );
                          ^
        In file included from podofo-trunk/src/base/PdfSigIncWriter.cpp:34:0:
        podofo-trunk/src/base/PdfSigIncWriter.h:50:18: warning:   by ‘virtual void PoDoFo::PdfSigIncWriter::Write(PoDoFo::PdfOutputDevice*, PoDoFo::pdf_int64)’ [-Woverloaded-virtual]
             virtual void Write( PdfOutputDevice* pDevice, pdf_int64 prevOffset);
                          ^

I guess the best option will be to rename the method in the
PdfSigIncWriter.h, thus the names, and the prototypes, will not clash.
This involves much more changes around the code. I cannot think of any
nice name for the rename, thus either IncWrite(...) or
WriteWithOffset(...), whichever sounds better to people.

By the way, please provide patches with full paths (call `svn diff` from
the root directory of the checkout) and merge all the changes into one
patch, not in series of patches - both is easier for maintaining the
changes. Thanks for your understanding.
	Bye,
	zyx


------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.
Get unparalleled scalability from the best Selenium testing platform available
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs
_______________________________________________
Podofo-users mailing list
Podofo-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/podofo-users

Reply via email to