Fix some compiler warnings for unused local variables

[stweil]

Instead of assigning the return value to an otherwise unused variable (which also raises a compiler warning), use a cast to void to explicitly indicate that the return value is intentionally ignored.

Calling system() without checking the return value does not raise a compiler warning, so don't add the cast for those calls.

[DanBloomberg]

Thanks. It all looks fine to me, but github security has a warning, and I don't know how much credibility should be given to that.

[DanBloomberg]

Also, using (void)fscanf() gives CMake ubuntu build warnings, such as

/home/runner/work/leptonica/leptonica/src/colormap.c:1803:11: warning: ignoring return value of ‘fscanf’, declared with attribute warn_unused_result [-Wunused-result]
 1803 |     (void)fscanf(fp, "Color    R-val    G-val    B-val   Alpha\n");

[stweil]

So my pull request does not work for Ubuntu. It took me some time to find the reason. On macOS the problem does not exist. On Debian and Ubuntu functions like system, fseek, ... use __attribute__((warn_unused_result)) if recent versions of gcc or clang are used, optimization is enabled and the compiler macro _FORTIFY_SOURCE > 0.

Leptonica builds use recent versions of gcc or clang with optimization. On Debian, _FORTIFY_SOURCE is not set by default, so those warnings are not triggered. But Ubuntu sets it to 2 or even 3 in recent versions, and that causes warnings which cannot be suppressed by the cast to void (see this rather old discussion).

So what can we do?

• Using dummy assignments like in the current code causes new confusing compiler warnings.
• _FORTIFY_SOURCE can be undefined before the first include statement, but that might disable fortify features which are wanted.
• Some people use statements like if (system("...")) {}.
• The return values can be handled.
• Leptonica could use its own new wrapper functions leptonica_system ... which don't cause a compiler warning.

I think I'll try a patch for the last alternative.

[DanBloomberg]

That seems like the most elegant approach. leptonica_system() or lept_system() (I prefer shorter) could go in utils1.c, which is the same file where lept_stderr() is defined.

[stweil]

That seems like the most elegant approach. leptonica_system() or lept_system() (I prefer shorter) could go in utils1.c, which is the same file where lept_stderr() is defined.

I now updated the pull request with lept_system in utils1.c. Please review whether anything should be changed.

We still need similar wrappers lept_fscanf, lept_fgets, ... Should those be added to utils2.c which already has lept_fopen?

[DanBloomberg]

Sorry -- this got buried in my 'stack'.

Do we still have this critical issue? And why has it only surfaced when the sprintf() is passed to system() within this new function? We got away with it before.

[stweil]

Sorry -- this got buried in my 'stack'.

Do we still have this critical issue? And why has it only surfaced when the sprintf() is passed to system() within this new function?

I think that we always had this "issue", ~but it's obviously only checked in pull requests~. And I don't consider it critical, because it only affect printsplitimage. Who cares that arguments given to that program are passed to the shell which then runs lpr?

So we can either ignore the warning or fix it by checking whether the printer argument is syntactically a valid printer name.

[stweil]

Dan, you'll see that "critical error" and some more here (only visible for project members).

[DanBloomberg]

Good to see that this is not a real problem!

I just remembered that we already have a wrapper for system(), It's in utils2.c: callSystemDebug(). We should use that one, because system() will crash on iphones.

[DanBloomberg]

Also, I have 3 programs that do this 'critical' thing that are NOT caught by the compiler. They are cleanpdf, compresspdf, croppdf. They do one little thing differently: they use lept_stderr() to print the command before running it. Can that possibly make a difference?

[DanBloomberg]

And the plot gets thicker. I call callSystemDebug in the library itself, in gplot.c, pdfio2.c and writefile.c.

[DanBloomberg]

Could the reason callSystemDebug doesn't trigger this warning be because it only runs if LeptDebugOK is TRUE, and by default it is FALSE?

[DanBloomberg]

Commit 0d44776 adds the callSystemDebug() wrapper to all calls to system().

[stweil]

Commit 0d44776 adds the callSystemDebug() wrapper to all calls to system().

That does not fix the "critical" code scanning alerts ("Uncontrolled data used in OS command").

[stweil]

I close this pull request because my suggested changes don't fix the compiler warnings for calls of functions with __attribute__((warn_unused_result)).