[scribus-dev] Can please someone teach me how to fix the current compiler/build warnings

Sun Dec 13 01:10:24 UTC 2015

From: scribus.user at gmail.com
Date: Fri, 11 Dec 2015 07:41:24 -0600
To: scribus-dev at lists.scribus.net
Subject: [scribus-dev] Can please someone teach me how to fix the current	compiler/build warnings

Just ping me off list /Kunda
GCC warnings

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L1080-L1087

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L1126-L1129

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L1180-L1182

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L1239-L1242

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L1247-L1250

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L1314-L1340

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L2398-L2401

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L2421-L2428

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L2438-L2445

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L2457-L2460

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L2465-L2468

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L2475-L2477

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L2514-L2520

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L2528-L2534

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L2544-L2551

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L2576-L2581

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L2936-L2939

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L2954-L2962

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L3028-L3031

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L3045-L3048

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L3058-L3061

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L3191-L3202

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L3215-L3218

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L3225-L3246

_______________________________________________
scribus-dev mailing list
scribus-dev at lists.scribus.net
http://lists.scribus.net/mailman/listinfo/scribus-dev
Kunda,

Here are suggestions for a few of them.

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L1080-L1087
/home/travis/build/scribusproject/scribus/scribus/text/storytext.cpp: In member function void StoryText::insertChars(int, QString, bool):
/home/travis/build/scribusproject/scribus/scribus/text/storytext.cpp:503:36: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
if (d->cursorPosition >= (pos + i)) {

pos and i are int
ScText_Shared.cursorPosition is uint

You have several choices:
* make pos and i into uint (if you can confirm that they will never be set negative at the bounds of the loop)
* add a cast: if (d->cursorPosition >= (unit) (pos + i)) { (this will work, but it is better to avoid casts because an incorrect cast can hide a real problem)
* make cursorPosition into an int. This is the least desirable choice because it will probably have repercussions throughout the code, plus cursorPosition probably can't be negative.

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L1126-L1129
/home/travis/build/scribusproject/scribus/scribus/fonts/sfnt.cpp: In function bool sfnt::hasLongLocaFormat(const QByteArray&):
/home/travis/build/scribusproject/scribus/scribus/fonts/sfnt.cpp:584:40: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
   qDebug() << "loca format:" << (void*)idxToLocFormat;

The line before is uint idxToLocFormat = word16(head, ttf_head_indexToLocFormat);
I think that the (void*) is bad. It idxToLocFormat is an unsigned integer, not a pointer. Probably removing the (void*) will stop the warning.

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L1180-L1182
/home/travis/build/scribusproject/scribus/scribus/fonts/scfontmetrics.cpp:46:13: warning: void readAdobeGlyphNames() declared static but never defined [-Wunused-function]
static void readAdobeGlyphNames();

Both uses of the function are in code that is commented out with #if 0.
The declaration should probably be removed or commented out.

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L1239-L1242
/home/travis/build/scribusproject/scribus/scribus/plugins/fileloader/scribus134format/scribus134format.cpp: In member function PageItem* Scribus134Format::pasteItem(ScribusDoc*, ScXmlStreamAttributes&, const QString&, PageItem::ItemKind, int):
/home/travis/build/scribusproject/scribus/scribus/plugins/fileloader/scribus134format/scribus134format.cpp:2513:9: warning: enumeration value NoteFrame�not handled in switch [-Wswitch]
  switch (pt)

This means that the "switch" is on an enumerated type, and not all of the enumeration values are handled in "case" clauses.
If a lot of values of not listed, it is probably intentional, and you can add a "default: break;" at the end to catch them.
If only one or two values are missing, it could be a bug where someone added a new value and didn't update all of the places that need to handle it,

The end of this particular "switch" has a half-dozen values that run "Q_ASSERT(false); break;".

You would need to read the code closely to see if NoteFrame
* should go into the group that calls "Q_ASSERT(false); break;"
* can be ignored with a "case PageItem::NoteFrame: break;"
* needs a new "case" statement with special code to handle it

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L1239-L1242
/home/travis/build/scribusproject/scribus/scribus/plugins/gettext/pdbim/pdbim.cpp: In member function void PdbIm::loadFile(QString):
/home/travis/build/scribusproject/scribus/scribus/plugins/gettext/pdbim/pdbim.cpp:100:48: warning: ignoring return value of size_t fread(void*, size_t, size_t, FILE*), declared with attribute warn_unused_result [-Wunused-result]
  fread( &m_header, PDB_HEADER_SIZE, 1, m_pdfp );

fread() returns the number of records that it read.
If fread() is unsuccessful, the data (m_header in this case) will be uninitialized and code that uses it could fail or behave unpredictably.

The parameters to fread() are buffer pointer, record size, number of records, file
I think that the code should be
  if ( fread( &m_header, PDB_HEADER_SIZE, 1, m_pdfp ) != 1 ) {
    ScMessageBox::warning( say that you could not read the file, similar to the message about not being able to open the file )
    return;
  }

In general, you should always check system calls and library calls, except maybe printf to the console.

Some people would write "(void) fread( &m_header, PDB_HEADER_SIZE, 1, m_pdfp );" which suppresses the error message but does not solve the underlying problem of a failed read.
I always check reads and seeks in my personal code.

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L2421-L2428
/home/travis/build/scribusproject/scribus/scribus/plugins/import/revenge/rawpainter.cpp: In member function virtual void RawPainter::setStyle(const WPXPropertyList&, const WPXPropertyListVector&):
/home/travis/build/scribusproject/scribus/scribus/plugins/import/revenge/rawpainter.cpp:2335:10: warning: variable angle�set but not used [-Wunused-but-set-variable]
   double angle = 0;

The variable angle is set to propList["draw:angle"]->getDouble() and then never used.
The choices are
* remove the declaration and the assignment (maybe the code was left over from a cut-and-paste from another block of code)
* check that this is not something that might be needed in the future. Maybe "angle" should be in RawPainter in plugins/import/revenge/rawpainter.h
* check that this is not the wrong name. Maybe "angle" should be "gradientAngle".

https://travis-ci.org/scribusproject/scribus/jobs/95496650#L2421-L2428
/home/travis/build/scribusproject/scribus/scribus/plugins/import/revenge/rawpainter.cpp: In member function ‘virtual void RawPainter::drawGraphicObject(const WPXPropertyList&, const WPXBinaryData&)’:
/home/travis/build/scribusproject/scribus/scribus/plugins/import/revenge/rawpainter.cpp:2953:20: warning: ‘ite’ may be used uninitialized in this function [-Wmaybe-uninitialized]
    applyShadow(ite);

I would initialize ite = NULL in the declaration at line 2848.
Then at the calls to applyFlip and applyShadow, I would write "if (ite) { applyFlip(ite); if (CurrColorFill != CommonStrings::None) applyShadow(ite); }"
That way, anyone reading the code later can easily see that 1) ite is always initialized and that 2) whatever happens in the big block of code, applyFlip() and applyShadow() will never be called unless they have an item.

Regards,William

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.scribus.net/pipermail/scribus-dev/attachments/20151212/44ebd425/attachment-0001.html>