Dne 1/25/2012 6:28 PM, Michal Hocko napsal(a):
Hi,
I have looked at the xpdf changes introduced by pdfeditor project
(http://code.google.com/p/pdfeditor/). 

I would like to clarify some of them before mergin:

diff --git a/src/xpdf/goo/GString.cc b/src/xpdf/goo/GString.cc
index bd36061..86ae2e9 100644
--- a/src/xpdf/goo/GString.cc
+++ b/src/xpdf/goo/GString.cc
@@ -229,9 +229,9 @@ GString *GString::appendfv(char *fmt, va_list argList) {
   int idx, width, prec;
   GBool reverseAlign, zeroFill;
   GStringFormatType ft;
-  char buf[65];
-  int len, i;
-  char *p0, *p1, *str;
+  char buf[65] = {};
+  int len=0, i=0;
+  char *p0 = NULL, *p1 = NULL, *str = NULL;
 
   argsLen = 0;
   argsSize = 8;

Is this just a compilation warning fix? The function is one giant mess
so I am not that surprised that this approach has been taken but at
least p0 doesn't need an explicit initialization as it is assigned from
fmt few lines bellow. 
p1 is the same case as it gets initialized from p0. 
str might be unitialized only if fmt contains an unexpected format
string so make it NULL will blow up which is good. So I would take that
part of the patch.

these are my changes, every initialization is better than none.


diff --git a/src/xpdf/xpdf/CharCodeToUnicode.cc b/src/xpdf/xpdf/CharCodeToUnicode.cc
index 71465f0..c7c9e3c 100644
--- a/src/xpdf/xpdf/CharCodeToUnicode.cc
+++ b/src/xpdf/xpdf/CharCodeToUnicode.cc
@@ -496,19 +496,19 @@ static bool isUnicodeSame(const Unicode *u1, int size1, const Unicode *u2, int s
 }
 
 CharCode CharCodeToUnicode::mapFromUnicode(const Unicode *u, int size)const {
-	int i;
+	unsigned int i;
 	if (size == 1) {
 		for (i = 0; i < mapLen; ++i)
 			if (map[i] == *u)
 				return i;
 	}
-	for (i = 0; i < sMapLen; ++i) {
+	for (i = 0; i < (unsigned int)sMapLen; ++i) {
 		if (isUnicodeSame(u, size, sMap[i].u, sMap[i].len))
 			return sMap[i].c;
 	}

If the sMapLen is negative for some readon we would loop for quite
some time ;) So this cannot be right. What is the reason for the
change? Compiler warning fix?

afaict, length(size) is always >= 0

 
 	// Nothing found
-	return -1;
+	return (CharCode)-1;
 }
 
 //------------------------------------------------------------------------
diff --git a/src/xpdf/xpdf/Function.cc b/src/xpdf/xpdf/Function.cc
index 5338b68..8241709 100644
--- a/src/xpdf/xpdf/Function.cc
+++ b/src/xpdf/xpdf/Function.cc
@@ -1093,7 +1093,7 @@ GBool PostScriptFunction::parseCode(Stream *str, int *codePtr) {
   char *p;
   GBool isReal;
   int opPtr, elsePtr;
-  int a, b, mid, cmp;
+  int a, b, mid, cmp = 0;

compiler warning fix - though I would rather see the variable being
moved to the proper place (into the block which uses it). But who
cares...
 
   while (1) {
     if (!(tok = getToken(str))) {
diff --git a/src/xpdf/xpdf/Gfx.cc b/src/xpdf/xpdf/Gfx.cc
index fe1c465..1a29de4 100644
--- a/src/xpdf/xpdf/Gfx.cc
+++ b/src/xpdf/xpdf/Gfx.cc
@@ -703,10 +703,8 @@ void Gfx::execOp(const Object *cmd, Object args[], int numArgs) {
 }
 
 Operator *Gfx::findOp(const char *name) {
-  int a, b, m, cmp;
+  int a = -1, b = numOps, m = 0, cmp = 0;
 
-  a = -1;
-  b = numOps;

I assume compiler warning but this one is really bogus because both m
and cmp will get assigned before they are used unconditionally.

   // invariant: opTab[a] < name < opTab[b]
   while (b - a > 1) {
     m = (a + b) / 2;
@@ -2127,7 +2125,7 @@ void Gfx::doAxialShFill(GfxAxialShading *shading) {
   double t0, t1, tt;
   double ta[axialMaxSplits + 1];
   int next[axialMaxSplits + 1];
-  GfxColor color0, color1;
+  GfxColor color0 = {}, color1 = {};
   int nComps;
   int i, j, k, kk;

What happens if we really do not initialize them? Isn't this just
papering over a real problem?
 
@@ -3150,7 +3148,7 @@ void Gfx::doShowText(const GString *s) {
   int wMode;
   double riseX, riseY;
   CharCode code;
-  Unicode u[8];
+  Unicode u[8] = {0};
   double x, y, dx, dy, dx2, dy2, curX, curY, tdx, tdy, lineX, lineY;
   double originX, originY, tOriginX, tOriginY;
   double oldCTM[6], newCTM[6];

Another initialized which is bogus but compilers are probably not clever
enough...

not bogus, this can be an error and further in that function is ""should"" be initialised by getNextChar but not every time!!! maybe it can is a real bug (afaicr, it really is) maybe not because of other variable holding additional information.

"""

Initializing arrays.

When declaring a regular array of local scope (within a function, for example), if we do not specify otherwise, its elements will not be initialized to any value by default, so their content will be undetermined until we store some value in them. The elements of global and static arrays, on the other hand, are automatically initialized with their default values, which for all fundamental types this means they are filled with zeros.
"""
http://www.cplusplus.com/doc/tutorial/arrays/


diff --git a/src/xpdf/xpdf/GfxFont.cc b/src/xpdf/xpdf/GfxFont.cc
index fb55d20..49ad825 100644
--- a/src/xpdf/xpdf/GfxFont.cc
+++ b/src/xpdf/xpdf/GfxFont.cc
@@ -168,7 +168,7 @@ void GfxFont::readFontDescriptor(XRef *xref, const Dict *fontDict) {
   int i;
 
   // assume Times-Roman by default (for substitution purposes)
-  flags = fontSerif;
+  flags = PDF_fontSerif;
 
   embFontID.num = -1;
   embFontID.gen = -1;

Any reason for rename? I do not see any reference to PDF_RENAMED_FONTS
anywhere in the tree. And I really don't like this.
[...]

@@ -1434,7 +1434,8 @@ int GfxCIDFont::getNextChar(const char *s, int len, CharCode *code,
   if (ctu) {
     *uLen = ctu->mapToUnicode(cid, u, uSize);
   } else {
-    *uLen = 0;
+	u[0] = *code;
+    *uLen = 1;
   }

This one looks like a fix but I would like to understand the case which
it fixes.

this is a real pdf fix when doing pdftotext it helps to get better results, otherwise you will get nothing.


 
   // horizontal
diff --git a/src/xpdf/xpdf/GfxFont.h b/src/xpdf/xpdf/GfxFont.h
index 556b7ee..b1afed6 100644
--- a/src/xpdf/xpdf/GfxFont.h
+++ b/src/xpdf/xpdf/GfxFont.h
@@ -79,11 +79,11 @@ struct GfxFontCIDWidths {
 // GfxFont
 //------------------------------------------------------------------------
 
-#define fontFixedWidth (1 << 0)
-#define fontSerif      (1 << 1)
-#define fontSymbolic   (1 << 2)
-#define fontItalic     (1 << 6)
-#define fontBold       (1 << 18)
+#define PDF_fontFixedWidth (1 << 0)
+#define PDF_fontSerif      (1 << 1)
+#define PDF_fontSymbolic   (1 << 2)
+#define PDF_fontItalic     (1 << 6)
+#define PDF_fontBold       (1 << 18)
 
 class GfxFont {
 public:
@@ -132,11 +132,11 @@ public:
 
   // Get font descriptor flags.
   int getFlags()const { return flags; }
-  GBool isFixedWidth()const { return flags & fontFixedWidth; }
-  GBool isSerif()const { return flags & fontSerif; }
-  GBool isSymbolic()const { return flags & fontSymbolic; }
-  GBool isItalic()const { return flags & fontItalic; }
-  GBool isBold()const { return flags & fontBold; }
+  GBool isFixedWidth()const { return flags & PDF_fontFixedWidth; }
+  GBool isSerif()const { return flags & PDF_fontSerif; }
+  GBool isSymbolic()const { return flags & PDF_fontSymbolic; }
+  GBool isItalic()const { return flags & PDF_fontItalic; }
+  GBool isBold()const { return flags & PDF_fontBold; }

I have already mentioned this above. NAK for this. It makes us diverge
from xpdf code base for no good reason.

we are already too far away.

 
   // Return the font matrix.
   const double *getFontMatrix()const { return fontMat; }
diff --git a/src/xpdf/xpdf/JPXStream.cc b/src/xpdf/xpdf/JPXStream.cc
index c0f4924..e8cbac1 100644
--- a/src/xpdf/xpdf/JPXStream.cc
+++ b/src/xpdf/xpdf/JPXStream.cc
@@ -876,10 +876,10 @@ GBool JPXStream::readColorSpecBox(Guint dataLen) {
 GBool JPXStream::readCodestream(Guint len) {
   JPXTile *tile;
   JPXTileComp *tileComp;
-  int segType;
+  int segType=0;
   GBool haveSIZ, haveCOD, haveQCD, haveSOT;
-  Guint precinctSize, style;
-  Guint segLen, capabilities, comp, i, j, r;
+  Guint precinctSize=0, style=0;
+  Guint segLen=0, capabilities=0, comp=0, i=0, j=0, r=0;
 
   //----- main header
   haveSIZ = haveCOD = haveQCD = haveSOT = gFalse;
@@ -1405,10 +1405,10 @@ GBool JPXStream::readTilePart() {
   GBool haveSOD;
   Guint tileIdx, tilePartLen, tilePartIdx, nTileParts;
   GBool tilePartToEOC;
-  Guint precinctSize, style;
-  Guint n, nSBs, nx, ny, sbx0, sby0, comp, segLen;
-  Guint i, j, k, cbX, cbY, r, pre, sb, cbi;
-  int segType, level;
+  Guint precinctSize=0, style=0;
+  Guint n=0, nSBs=0, nx=0, ny=0, sbx0=0, sby0=0, comp=0, segLen=0;
+  Guint i=0, j=0, k=0, cbX=0, cbY=0, r=0, pre=0, sb=0, cbi=0;
+  int segType=0, level=0;
 
   // process the SOT marker segment
   if (!readUWord(&tileIdx) ||

Oh well, another compiler shut up changes. I haven't checked whether
this is correct or it just papers over a real issue but I guess we can
apply this...

diff --git a/src/xpdf/xpdf/Lexer.cc b/src/xpdf/xpdf/Lexer.cc
index 3217b24..d36f9f3 100644
--- a/src/xpdf/xpdf/Lexer.cc
+++ b/src/xpdf/xpdf/Lexer.cc
@@ -23,7 +23,7 @@
 
 // A '1' in this array means the character is white space.  A '1' or
 // '2' means the character ends a name or command.
-char specialChars[256] = {
+static char specialChars[256] = {
   1, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 1, 1, 0, 0,   // 0x
   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,   // 1x
   1, 0, 0, 0, 0, 2, 0, 0, 2, 2, 0, 0, 0, 0, 0, 2,   // 2x
diff --git a/src/xpdf/xpdf/Lexer.h b/src/xpdf/xpdf/Lexer.h
index 05fd708..09a248b 100644
--- a/src/xpdf/xpdf/Lexer.h
+++ b/src/xpdf/xpdf/Lexer.h
@@ -20,7 +20,6 @@
 
 class XRef;
 
-extern char specialChars[256];
 #define tokBufSize 128		// size of token buffer

No we cannot do that. This would break 2eb4527: kernel: fix makeNamePdfValid


 //------------------------------------------------------------------------
diff --git a/src/xpdf/xpdf/SplashOutputDev.cc b/src/xpdf/xpdf/SplashOutputDev.cc
index 3c78768..b7fea5c 100644
--- a/src/xpdf/xpdf/SplashOutputDev.cc
+++ b/src/xpdf/xpdf/SplashOutputDev.cc
@@ -959,7 +959,18 @@ void SplashOutputDev::updateStrokeOpacity(GfxState *state) {
 void SplashOutputDev::updateFont(GfxState *state) {
   needFontUpdate = gTrue;
 }
-
+SplashFont * SplashOutputDev::getFontById(GfxFont * gfxFont)
+{
+	SplashOutFontFileID *id;
+	id = new SplashOutFontFileID(gfxFont->getID());
+	SplashFontFile *fontFile = fontEngine->getFontFile(id);
+	delete id;
+	if (!fontFile )
+		return NULL;
+	SplashCoord coord[] = {1,0,0,1,0,0};
+	SplashFont * fnt = fontEngine->getFont(fontFile,coord,coord);
+	return fnt;
+}

What about other OutputDevices? E.g TextOutputDev.

 void SplashOutputDev::doUpdateFont(GfxState *state) {
   const GfxFont *gfxFont;
   GfxFontType fontType;
@@ -2599,7 +2610,7 @@ void SplashOutputDev::setSoftMask(GfxState *state, const double *bbox,
 #if SPLASH_CMYK
   GfxCMYK cmyk;
 #endif
-  double lum, lum2;
+  double lum=0.0, lum2=0.0;
   int tx, ty, x, y;
 
   tx = transpGroupStack->tx;
@@ -2759,7 +2770,7 @@ void SplashOutputDev::setFillColor(int r, int g, int b) {
 #endif
 }
 
-SplashFont *SplashOutputDev::getFont(GString *name, double *textMatA) {
+SplashFont *SplashOutputDev::getFont(const GString *name, double *textMatA) {
   DisplayFontParam *dfp;
   Ref ref;
   SplashOutFontFileID *id;

OK


[...]
diff --git a/src/xpdf/xpdf/Stream.cc b/src/xpdf/xpdf/Stream.cc
index f72c22a..a6407b3 100644
--- a/src/xpdf/xpdf/Stream.cc
+++ b/src/xpdf/xpdf/Stream.cc
@@ -631,7 +631,7 @@ Stream * FileStream::clone()
    long startPos=ftell(f);
 
    if(limited && !l) 
-           printf("%s: limited stream with 0 lenght\n", __FUNCTION__);
+           error( currPos, "%s: limited stream with 0 lenght\n", __FUNCTION__);
 
    // if length is 0, calculates it until end of file
    if(!limited && !l)

OK

So I will merge fixes which are obviously right and keep those that
needs some answers for later.

Thanks.