DanBloomberg / leptonica

Leptonica is an open source library containing software that is broadly useful for image processing and image analysis applications. The official github repository for Leptonica is: danbloomberg/leptonica. See leptonica.org for more documentation.
Other
1.72k stars 384 forks source link

1.84.1: compile time warnings #726

Open kloczek opened 6 months ago

kloczek commented 6 months ago

First stats:

$ rpmbuild -ba leptonica.spec 2>&1 | grep -- \\[-W | sed 's/.*\[//; s/\]//' | sort | uniq -c | sort -nr
     36 -Wunused-variable
     32 -Wunused-but-set-variable
      2 -Wunused-function
      1 -Wuse-after-free
      1 -Wunused-const-variable=
      1 -Wmisleading-indentation
And here is extracted stderr ```console readbarcode.h:198:20: warning: 'Code128' defined but not used [-Wunused-variable] 198 | static const char *Code128[] = { | ^~~~~~~ colormap.c: In function 'pixcmapReadStream': colormap.c:1790:35: warning: variable 'ignore' set but not used [-Wunused-but-set-variable] 1790 | l_int32 rval, gval, bval, aval, ignore; | ^~~~~~ fhmtgenlow.1.c: In function 'fhmt_1_5': fhmtgenlow.1.c:273:21: warning: variable 'wpls5' set but not used [-Wunused-but-set-variable] 273 | l_int32 wpls5, wpls6; | ^~~~~ fhmtgenlow.1.c:272:35: warning: variable 'wpls4' set but not used [-Wunused-but-set-variable] 272 | l_int32 wpls2, wpls3, wpls4; | ^~~~~ fhmtgenlow.1.c:272:28: warning: variable 'wpls3' set but not used [-Wunused-but-set-variable] 272 | l_int32 wpls2, wpls3, wpls4; | ^~~~~ gplot.c: In function 'gplotRead': gplot.c:1209:35: warning: variable 'ignore' set but not used [-Wunused-but-set-variable] 1209 | l_int32 outformat, ret, version, ignore; | ^~~~~~ gplot.c:1208:47: warning: variable 'ignores' set but not used [-Wunused-but-set-variable] 1208 | char *rootname, *title, *xlabel, *ylabel, *ignores; | ^~~~~~~ In file included from alltypes.h:37, from allheaders.h:35, from gplot.c:153: gplot.c: In function 'gplotGenCommandFile': environ.h:583:32: warning: pointer 'cmdstr_88' may be used after 'free' [-Wuse-after-free] 583 | IF_SEV(L_SEVERITY_ERROR, returnErrorInt1((a), (f), (b), (c)), \ | ^ environ.h:379:44: note: call to 'free' here 379 | #define LEPT_FREE(ptr) free(ptr) | ^~~~~~~~~ kernel.c: In function 'kernelReadStream': kernel.c:540:48: warning: variable 'ignore' set but not used [-Wunused-but-set-variable] 540 | l_int32 sy, sx, cy, cx, i, j, ret, version, ignore; | ^~~~~~ partify.c: In function 'boxaRemoveVGaps': partify.c:306:5: warning: this 'if' clause does not guard... [-Wmisleading-indentation] 306 | if ((nbox = boxaGetCount(boxa)) == 0); | ^~ partify.c:307:9: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if' 307 | return ERROR_INT("boxa is empty", __func__, 1); | ^~~~~~ rop.c:541:1: warning: 'checkRasteropCrop' defined but not used [-Wunused-function] 541 | checkRasteropCrop(l_int32 pixw, | ^~~~~~~~~~~~~~~~~ pnmio.c: In function 'pnmReadNextAsciiValue': pnmio.c:1330:10: warning: unused variable 'ignore' [-Wunused-variable] 1330 | l_int32 ignore; | ^~~~~~ In file included from readbarcode.c:89: readbarcode.h:198:20: warning: 'Code128' defined but not used [-Wunused-variable] 198 | static const char *Code128[] = { | ^~~~~~~ readbarcode.h:184:20: warning: 'Upca' defined but not used [-Wunused-variable] 184 | static const char *Upca[] = { | ^~~~ readbarcode.h:169:20: warning: 'Codabar' defined but not used [-Wunused-variable] 169 | static const char *Codabar[] = { | ^~~~~~~ readbarcode.h:144:20: warning: 'Code39' defined but not used [-Wunused-variable] 144 | static const char *Code39[] = { | ^~~~~~ readbarcode.h:120:20: warning: 'Code93' defined but not used [-Wunused-variable] 120 | static const char *Code93[] = { | ^~~~~~ readbarcode.h:107:20: warning: 'CodeI2of5' defined but not used [-Wunused-variable] 107 | static const char *CodeI2of5[] = { | ^~~~~~~~~ readbarcode.h:93:20: warning: 'Code2of5' defined but not used [-Wunused-variable] 93 | static const char *Code2of5[] = { | ^~~~~~~~ readbarcode.h:78:21: warning: 'SupportedBarcodeFormatName' defined but not used [-Wunused-variable] 78 | static const char *SupportedBarcodeFormatName[] = { | ^~~~~~~~~~~~~~~~~~~~~~~~~~ pngio.c: In function 'pixReadStreamPng': pngio.c:191:30: warning: variable 'ncolors' set but not used [-Wunused-but-set-variable] 191 | l_int32 i, j, k, index, ncolors, rval, gval, bval, valid; | ^~~~~~~ pngio.c: In function 'pixReadMemPng': pngio.c:1576:30: warning: variable 'ncolors' set but not used [-Wunused-but-set-variable] 1576 | l_int32 i, j, k, index, ncolors, rval, gval, bval, valid; | ^~~~~~~ sarray1.c: In function 'sarrayReadStream': sarray1.c:1390:47: warning: variable 'ignore' set but not used [-Wunused-but-set-variable] 1390 | l_int32 i, n, size, index, bufsize, version, ignore, success; | ^~~~~~ sarray1.c: In function 'getFilenamesInDirectory': sarray1.c:1888:17: warning: unused variable 'dfd' [-Wunused-variable] 1888 | int dfd, stat_ret; | ^~~ sel1.c: In function 'selReadStream': sel1.c:1354:41: warning: variable 'ignore' set but not used [-Wunused-but-set-variable] 1354 | l_int32 sy, sx, cy, cx, i, j, version, ignore; | ^~~~~~ utils2.c: In function 'callSystemDebug': utils2.c:2673:10: warning: variable 'ret' set but not used [-Wunused-but-set-variable] 2673 | l_int32 ret; | ^~~ blend2_reg.c:42:19: warning: ‘fname_bmp’ defined but not used [-Wunused-const-variable=] 42 | static const char fname_bmp[64] = "/tmp/lept/regout/blend2.14.bmp"; | ^~~~~~~~~ In file included from autogentest2.c:42: autogen.137.h:155:20: warning: ‘l_strdata_1’ defined but not used [-Wunused-variable] 155 | static const char *l_strdata_1 = | ^~~~~~~~~~~ autogen.137.h:54:20: warning: ‘l_strdata_0’ defined but not used [-Wunused-variable] 54 | static const char *l_strdata_0 = | ^~~~~~~~~~~ In file included from barcodetest.c:43: ../src/readbarcode.h:198:20: warning: ‘Code128’ defined but not used [-Wunused-variable] 198 | static const char *Code128[] = { | ^~~~~~~ ../src/readbarcode.h:184:20: warning: ‘Upca’ defined but not used [-Wunused-variable] 184 | static const char *Upca[] = { | ^~~~ ../src/readbarcode.h:169:20: warning: ‘Codabar’ defined but not used [-Wunused-variable] 169 | static const char *Codabar[] = { | ^~~~~~~ ../src/readbarcode.h:144:20: warning: ‘Code39’ defined but not used [-Wunused-variable] 144 | static const char *Code39[] = { | ^~~~~~ ../src/readbarcode.h:120:20: warning: ‘Code93’ defined but not used [-Wunused-variable] 120 | static const char *Code93[] = { | ^~~~~~ ../src/readbarcode.h:107:20: warning: ‘CodeI2of5’ defined but not used [-Wunused-variable] 107 | static const char *CodeI2of5[] = { | ^~~~~~~~~ ../src/readbarcode.h:93:20: warning: ‘Code2of5’ defined but not used [-Wunused-variable] 93 | static const char *Code2of5[] = { | ^~~~~~~~ ../src/readbarcode.h:78:21: warning: ‘SupportedBarcodeFormatName’ defined but not used [-Wunused-variable] 78 | static const char *SupportedBarcodeFormatName[] = { | ^~~~~~~~~~~~~~~~~~~~~~~~~~ cleanpdf.c: In function ‘main’: cleanpdf.c:155:63: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable] 155 | l_int32 i, n, res, contrast, rotation, opensize, render_res, ret; | ^~~ compresspdf.c: In function ‘main’: compresspdf.c:133:65: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable] 133 | l_int32 imres, render_res, onebit, savecolor, quality, i, n, ret; | ^~~ croppdf.c: In function ‘main’: croppdf.c:103:30: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable] 103 | l_int32 render_res, i, n, ret; | ^~~ livre_makefigs.c: In function ‘main’: livre_makefigs.c:46:10: warning: variable ‘ignore’ set but not used [-Wunused-but-set-variable] 46 | l_int32 ignore; | ^~~~~~ hashtest.c: In function ‘BuildShortStrings’: hashtest.c:261:11: warning: unused variable ‘hash’ [-Wunused-variable] 261 | l_uint64 hash; | ^~~~ messagetest.c: In function ‘TestMessageControl’: messagetest.c:138:10: warning: variable ‘orig_severity’ set but not used [-Wunused-but-set-variable] 138 | l_int32 orig_severity; | ^~~~~~~~~~~~~ messagetest.c: In function ‘TestStderrRedirect’: messagetest.c:160:7: warning: variable ‘pix1’ set but not used [-Wunused-but-set-variable] 160 | PIX *pix1; | ^~~~ rasteroptest.c: In function ‘main’: rasteroptest.c:59:22: warning: variable ‘selectop’ set but not used [-Wunused-but-set-variable] 59 | l_int32 niters, op, selectop; | ^~~~~~~~ printtiff.c: In function ‘main’: printtiff.c:67:14: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable] 67 | l_int32 ret; | ^~~ printimage.c: In function ‘main’: printimage.c:89:12: warning: unused variable ‘fp’ [-Wunused-variable] 89 | FILE *fp; | ^~ printimage.c:87:21: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable] 87 | l_int32 i, w, h, ret, index; | ^~~ printsplitimage.c: In function ‘main’: printsplitimage.c:78:14: warning: unused variable ‘fp’ [-Wunused-variable] 78 | FILE *fp; | ^~ printsplitimage.c:76:42: warning: variable ‘ignore’ set but not used [-Wunused-but-set-variable] 76 | l_int32 nx, ny, i, w, h, ws, hs, n, ignore, index; | ^~~~~~ rotateorthtest1.c: In function ‘main’: rotateorthtest1.c:50:12: warning: unused variable ‘pops’ [-Wunused-variable] 50 | l_float32 pops; | ^~~~ rotateorthtest1.c:49:26: warning: unused variable ‘pixt’ [-Wunused-variable] 49 | PIX *pixs, *pixd, *pixt; | ^~~~ rotatetest1.c: In function ‘main’: rotatetest1.c:44:21: warning: variable ‘fileout’ set but not used [-Wunused-but-set-variable] 44 | char *filein, *fileout; | ^~~~~~~ rotatetest1.c:43:12: warning: variable ‘angle’ set but not used [-Wunused-but-set-variable] 43 | l_float32 angle, deg2rad, ang; | ^~~~~ recog_bootnum1.c:281:7: warning: ‘MakeBootnum2’ defined but not used [-Wunused-function] 281 | PIXA *MakeBootnum2(void) | ^~~~~~~~~~~~ In file included from recog_bootnum1.c:62: ../src/bmfdata.h:55:21: warning: ‘outputfonts’ defined but not used [-Wunused-variable] 55 | static const char *outputfonts[] = {"chars-4.pa", "chars-6.pa", | ^~~~~~~~~~~ ../src/bmfdata.h:50:21: warning: ‘inputfonts’ defined but not used [-Wunused-variable] 50 | static const char *inputfonts[] = {"chars-4.tif", "chars-6.tif", | ^~~~~~~~~~ seedfilltest.c: In function ‘main’: seedfilltest.c:52:57: warning: unused variable ‘pixt3’ [-Wunused-variable] 52 | PIX *pixs, *pixd, *pixm, *pixmi, *pixt1, *pixt2, *pixt3; | ^~~~~ seedfilltest.c:52:49: warning: unused variable ‘pixt2’ [-Wunused-variable] 52 | PIX *pixs, *pixd, *pixm, *pixmi, *pixt1, *pixt2, *pixt3; | ^~~~~ seedfilltest.c:52:41: warning: unused variable ‘pixt1’ [-Wunused-variable] 52 | PIX *pixs, *pixd, *pixm, *pixmi, *pixt1, *pixt2, *pixt3; | ^~~~~ seedfilltest.c:51:12: warning: variable ‘size’ set but not used [-Wunused-but-set-variable] 51 | l_float32 size; | ^~~~ seedfilltest.c:48:21: warning: variable ‘fileout’ set but not used [-Wunused-but-set-variable] 48 | char *filein, *fileout; | ^~~~~~~ sheartest.c: In function ‘main’: sheartest.c:49:35: warning: unused variable ‘pixd’ [-Wunused-variable] 49 | PIX *pixt1, *pixt2, *pixs, *pixd; | ^~~~ sheartest.c:48:19: warning: variable ‘deg2rad’ set but not used [-Wunused-but-set-variable] 48 | l_float32 angle, deg2rad; | ^~~~~~~ sheartest.c:48:12: warning: variable ‘angle’ set but not used [-Wunused-but-set-variable] 48 | l_float32 angle, deg2rad; | ^~~~~ sheartest.c:47:28: warning: unused variable ‘linex’ [-Wunused-variable] 47 | l_int32 i, w, h, liney, linex, same; | ^~~~~ sheartest.c:47:21: warning: unused variable ‘liney’ [-Wunused-variable] 47 | l_int32 i, w, h, liney, linex, same; | ^~~~~ sheartest.c:47:12: warning: unused variable ‘i’ [-Wunused-variable] 47 | l_int32 i, w, h, liney, linex, same; | ^ sheartest.c:46:21: warning: variable ‘fileout’ set but not used [-Wunused-but-set-variable] 46 | char *filein, *fileout; | ^~~~~~~ sorttest.c: In function ‘main’: sorttest.c:45:10: warning: unused variable ‘i’ [-Wunused-variable] 45 | l_int32 i, n, ns; | ^ splitpdf.c: In function ‘main’: splitpdf.c:58:32: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable] 58 | l_int32 i, n, npages, nfiles, ret, val, start, end; | ^~~ tiffpdftest.c: In function ‘main’: tiffpdftest.c:48:12: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable] 48 | l_int32 ret; | ^~~ warpertest.c: In function ‘main’: warpertest.c:56:27: warning: variable ‘index’ set but not used [-Wunused-but-set-variable] 56 | l_int32 w, h, i, j, k, index, op, dir, stretch; | ^~~~~ ```
DanBloomberg commented 6 months ago

Thank you for looking at this. I am impressed with your command :-)

I happened to go through this list a few days ago. With one notable exception these are not problematic. The unused variables are behind preprocessor walls, so the compiler doesn't see their use. The set but unused variables are mostly there to avoid other warnings about ignoring returned data; many of these variables are called "ignore". The 2 unused functions have been used in the past and can be used for debugging or making some data if necessary.

The misleading indentation is a bug I introduced (an extra ';') while fixing stuff yesterday (!) It's in a unit (partify) that is not working properly yet, so this isn't important.

Finally, the use after free is an error in gplot.c. It's in a section of code that is only invoked if there is an error opening a stream, which would be very rare. This will be fixed in a day or two.

stweil commented 6 months ago

Even more warnings are produced with clang and compiler option -Weverything:

  18 [-Waddress-of-packed-member]
  96 [-Watomic-implicit-seq-cst]
  39 [-Wbad-function-cast]
  11 [-Wcast-align]
  31 [-Wcast-qual]
   3 [-Wcomma]
 189 [-Wconditional-uninitialized]
   3 [-Wdeclaration-after-statement]
  18 [-Wdisabled-macro-expansion]
   5 [-Wdocumentation-html]
   4 [-Wdocumentation-unknown-command]
1887 [-Wdocumentation]
2118 [-Wdouble-promotion]
   1 [-Wempty-translation-unit]
 158 [-Wfloat-conversion]
  35 [-Wfloat-equal]
   4 [-Wformat-nonliteral]
   5 [-Wformat-pedantic]
1008 [-Wimplicit-float-conversion]
 554 [-Wimplicit-int-conversion]
   1 [-Wliteral-conversion]
   1 [-Wliteral-range]
   2 [-Wmissing-noreturn]
   2 [-Wmissing-prototypes]
  30 [-Wmissing-variable-declarations]
  11 [-Woverlength-strings]
  46 [-Wpadded]
  11 [-Wshadow]
   1 [-Wshift-sign-overflow]
 165 [-Wshorten-64-to-32]
 171 [-Wsign-compare]
1080 [-Wsign-conversion]
   7 [-Wstrict-prototypes]
   1 [-Wunaligned-access]
  13 [-Wundef]
   9 [-Wunreachable-code-break]
   1 [-Wunreachable-code-return]
   8 [-Wunreachable-code]
  25 [-Wunused-but-set-variable]
   1 [-Wunused-const-variable]
   2 [-Wunused-function]
  33 [-Wunused-macros]
 127 [-Wunused-parameter]
  15 [-Wunused-variable]

Some of them are worth being fixed, for example unnecessary conversions between float and double which cost a little bit of performance.

DanBloomberg commented 6 months ago

That list looks like a warning not to get out of bed in the morning because 1000 bad things could happen. Or even worse, you might waste a year of your life chasing down compiler warnings ;-)

Never thought about overhead from float to double conversion. I have used float wherever possible because it saves storage and it used to be faster. So only used double if the precision was necessary (e.g., representing 32 bit integers, transforms near singularities, hashing functions, etc).