💾 Archived View for thrig.me › blog › 2024 › 03 › 18 › re-linters.gmi captured on 2024-08-18 at 18:13:59. Gemini links have been rewritten to link to archived content
⬅️ Previous capture (2024-03-21)
-=-=-=-=-=-=-
gemini://degrowther.smol.pub/20240316_linters
Compiler warnings have gotten better so cranking up the warnings is worth some effort.
$ make player.o
cc -O2 -pipe -std=c99 -fno-diagnostics-color \
-fstack-protector-strong -pipe -pedantic -Wall -Wextra -Wcast-qual \
-Wconversion -Wformat-security -Wformat=2 -Wno-unused-function \
-Wno-unused-parameter -Wnull-dereference -Wpointer-arith -Wshadow \
-Wstack-protector -Wstrict-overflow=2 -c player.c -o player.o
player.c:221:14: warning: equality comparison result unused
[-Wunused-comparison]
app->move == ERR;
~~~~~~~~~~^~~~~~
player.c:221:14: note: use '=' to turn this equality comparison into
an assignment
app->move == ERR;
^~
=
1 warning generated.
That's an error by me from, uh, Friday. The warnings were being ignored on account of playtesting, which is quite a different state of mind from a "looking for errors" critical review, and being distracted by warnings may knock you out of the playtesting state. Ideally you switch between these modes now and then, or maybe you actually have other people who can take a glance at the code and give you a "wtf/minute" review. Tests, etc.
There can easily be more noise than signal, depending on the tool or the settings. For example, here's GCC with -Wstrict-overflow=3 set.
player.c: In function 'player_update':
player.c:563:1: warning: assuming signed overflow does not occur
when changing X +- C1 cmp C2 to X cmp C2 -+ C1 [-Wstrict-overflow]
563 | }
| ^
player.c:563:1: warning: assuming signed overflow does not occur
when changing X +- C1 cmp C2 to X cmp C2 -+ C1 [-Wstrict-overflow]
player.c:563:1: warning: assuming signed overflow does not occur
when changing X +- C1 cmp C2 to X cmp C2 -+ C1 [-Wstrict-overflow]
player.c:563:1: warning: assuming signed overflow does not occur
when changing X +- C1 cmp C2 to X cmp C2 -+ C1 [-Wstrict-overflow]
player.c:563:1: warning: assuming signed overflow does not occur
when changing X +- C1 cmp C2 to X cmp C2 -+ C1 [-Wstrict-overflow]
player.c: In function 'play_update':
player.c:563:1: warning: assuming signed overflow does not occur
when changing X +- C1 cmp C2 to X cmp C2 -+ C1 [-Wstrict-overflow]
Eeeeeh? Line 563 is the last "}" in the file, closing a function that returns void, and why the same thing six times? I did not feel up to wrestling with a search engine nor digging through the GCC source code, so -Wstrict-overflow=3 got changed to -Wstrict-overflow=2.
If there are too many warnings, most humans will tune all the warnings out. A coworker was surprised when their Nagios messages were ignored by everyone else; they had Nagios generating something like 10,000 emails per year—about 30 per day, and a welcome basket of somewhere between 60 to 120 on enjoy your Monday! (If you didn't have to come in on a Saturday, if you know what I mean.) The number of errors worthy of attention was somewhere below ten in any given year. Even a 50% noise to important message rate is way too high: a coinflip says it's noise that will waste my time. Is 10% noise too high? Boy who cried wolf and all that.
Another problem is that the warnings may simply be papered over without understanding the issue. This could introduce security errors.
And there is the "Hell is other people's code" problem where there are pages and pages of warnings. There may not be much motivation to go through and fix them, as you may have no clue what the code does nor what the programmer was trying to do by doing math on NULL which at least there is (now) a warning for. Mmmmmm, undefined behavior!
tcod.c:495:27: warning: performing pointer arithmetic on a null
pointer has undefined behavior [-Wnull-pointer-arithmetic]
return ((void **) NULL) - 1;
~~~~~~~~~~~~~~~~ ^
Maybe the code needs a particular quirk from a particular piece of hardware or operating system or compiler to work correctly? Good luck!
void **
TCOD_list_remove_iterator_fast(TCOD_list_t l, void **elt)
{
*elt = l->array[l->fillSize - 1];
l->fillSize--;
if (l->fillSize == 0) {
return ((void **) NULL) - 1;
} else {
return elt - 1;
}
}
This library exhibited mysterious memory errors on OpenBSD, so I ended up not using it.
Linters can shade from "that's an error" to "that form is dangerous" to "that form is dubious" to "you must indent with three spaces, as is only correct" pretty quickly. Some degree of formalism is inevitable in larger organizations or projects where consistency is desired. On the other hand, a flurry of whitespace patches as some new style comes into fashion is not unknown, e.g. shortly after "Perl Best Practices" was published.
$ perl -Mwarnings -E 'my $x; say 42 if $x = 99'
Found = in conditional, should be == at -e line 1.
42
$ cat script
open my $fh, "< /etc/passwd"
$ perl -Mwarnings ./script
$ perlcritic --theme community ./script
open() called with less than 3 arguments at line 1, column 1. The
one- and two-argument forms of open() parse functionality from the
filename, use the three-argument form instead. (Severity: 3)
Missing strict or warnings at line 1, column 1. The strict and
warnings pragmas are important to avoid common pitfalls and
deprecated/experimental functionality. Make sure each script or
module contains "use strict; use warnings;" or a module that does
this for you. (Severity: 4)
That's a bit more actionable than something about "X +- C1 cmp C2 to X cmp C2 -+ C1" on the last "}" in a file six times.
"linter" is being used in a vague sense here; there can be overlap between a compiler and third-party tools like lint(1) of yore, or editors of excessive size. In the olden times, the computer may have lacked the memory to run a compiler and a linter at the same time, and maybe they did not know what to warn about. These days, a compiler may handle some or all the tasks of warnings, tests, code coverage, applying the one true indentation style, spying on you, version requirements, etc., and we may have better ideas of what is important to warn about based on years of humans screwing things up.
$ perl -Mwarnings -e 'printf "%d\n", 42, 43'
Redundant argument in printf at -e line 1.
42
$ cfu 'printf("%d\n", 42, 43)'
/tmp/cfu.GngLZ5tkzd/cfu.c:52:21: warning: data argument not used by
format string [-Wformat-extra-args]
printf("%d\n", 42, 43);
~~~~~~ ^
1 warning generated.
42
$ sbcl --noinform --quit --eval '(format t "~a~&" 42 43)'
42
$ sbcl --noinform --quit --eval '(format t "~a~&")'
debugger invoked on a SB-FORMAT:FORMAT-ERROR in thread
#<THREAD "main thread" RUNNING {1003AD8003}>:
error in FORMAT: No more arguments
~a~&
^
...
So I do use linters, generally what the compiler provides, but tend not to turn the warnings up too high. I do not much use third-party tools except for indenters (cl-indentify, perltidy) where that is not provided by the language (Go) or a compiler (clang-format). Granted, I'm not collaborating with anyone so can get away with idiosyncratic styles and various bad practices. A professional may need to do things different?