[poppler] Compiling poppler with clang

[Albert Astals Cid]

El Diumenge, 26 d'agost de 2012, a les 08:08:37, Thomas Freitag va escriure:
> Hi all!
> 
> To detect the problems sent by Mateusz "j00ru" Jurczyk and Gynvael
> Coldwind I need the address sanitizer from clang, and therefore I
> compiled poppler the first time with clang. Here I got several warnings
> about unclean code, some of them sound funny, some sound serious. I
> would like to categorize them in these points:
> 
> 1. Useless if statements like
> gmem.cc:112:12: warning: comparison of unsigned expression < 0 is always
> false [-Wtautological-compare]
>    if (size < 0) {
>        ~~~~ ^ ~
> 
> 2. Unused private fields
> In file included from Gfx.cc:74:
> ../Gfx.h:222:10: warning: private field 'textClipBBox' is not used
> [-Wunused-private-field]
>    double textClipBBox[4];       // text clipping bounding box
>           ^
> 1 warning generated.
> 
> 3. Comparison of distinct pointer types like
> FoFiType1.cc:78:28: warning: comparison of distinct pointer types ('char
> **' and 'const char **') uses non-standard composite pointer type 'const
> char *const *'
>    if (encoding && encoding != fofiType1StandardEncoding) {
>                    ~~~~~~~~ ^  ~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> 4. Memory alignment problems
> CairoOutputDev.cc:1610:29: warning: cast from 'unsigned char *' to
> 'uint32_t *' (aka 'unsigned int *') increases required alignment from 1
> to 4 [-Wcast-align]
>      uint32_t *source_data =
> (uint32_t*)cairo_image_surface_get_data(source);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> 5. vtable pointer will be overwritten
> Function.cc:422:10: warning: destination for this 'memcpy' call is a
> pointer to dynamic class 'SampledFunction'; vtable pointer will be
> overwritten [-Wdynamic-class-memaccess]
>    memcpy(this, func, sizeof(SampledFunction));
>    ~~~~~~ ^
> Function.cc:422:10: note: explicitly cast the pointer to silence this
> warning
> 
> At least categrory 5. sound serious to me, I would never have copied
> instances of C++ objects in that way, because it depends on the compiler
> and the class if this causes problems on runtime, s. i.e.
> http://weseetips.com/tag/afx_zero_init_object/, 

Note this is memset-ing to 0, not memcpy-ing a class to itself. To be honest i 
agree memcpy'in a SampledFunction to a SampledFunction is ugly, but i fail to 
see why it would not work.

> and some other projects
> like chromium already repaired that in the usual way, s.
> http://code.google.com/p/chromium/issues/detail?id=92756
> 
> What are You thinking about clean up poppler code to avoid these
> warings? I attach the complete stderr log from make.
> 
> @Albert: BTW, do You want a patch for every PDF document from Mateusz
> "j00ru" Jurczyk and Gynvael Coldwind, or can I collect them in the way I
> work on them?

I'd prefer a patch for pdf, makes it much more easy to review (and put in git) 
than a big patch adding ifs and checks everywhere.

Cheers,
  Albert

> 
> Cheers,
> Thomas