Skip to content

configure: enable -Wextra, fix the warnings it finds - #143

Closed
Pace17881 wants to merge 1 commit into
FreeSpacenav:masterfrom
Pace17881:build/enable-wextra
Closed

Pace17881 wants to merge 1 commit into
FreeSpacenav:masterfrom
Pace17881:build/enable-wextra

Conversation

@Pace17881

Copy link
Copy Markdown

Adds `-Wextra` alongside the existing `-pedantic -Wall` in `configure`, and fixes every warning it surfaces - checked both with X11 and with `--disable-x11`. No behavior changes; every fix is either an explicit cast where a comparison mixes signed/unsigned expressions that are always non-negative in practice, or a `(void)` cast on a parameter/variable a particular build configuration genuinely doesn't use. Details are in the commit message.

Two warnings remain deliberately out of scope for this PR:

Kept intentionally small/additive - no new files, no new tooling, just stricter use of the compiler already in use. Happy to adjust anything.

Adds -Wextra alongside the existing -pedantic -Wall in configure, and
fixes every warning it surfaces (checked with and without --disable-x11):

- sign-compare: cfgfile.c/dev.c/proto_unix.c compared signed and unsigned
  int expressions that are always non-negative in practice (USB vendor/
  product IDs, an accumulated byte count bounded by a struct size) -
  added casts, or in one spot in dev.c removed a stray one-sided cast
  that had only moved the mismatch to the other operand.
- missing-field-initializer: dev.c's usbdb[] sentinel entry only
  initialized 3 of 4 struct fields; made it explicit like every other
  row in the table.
- old-style-declaration: event.c had "const static" instead of
  "static const".
- unused-parameter: a handful of stub/fallback functions (kbemu.c's
  dummy_send*, kbemu_x11.c's non-X11 keyname stub, proto_x11.c's Xlib
  IO-error callback, cfgfile.c's non-X11 parse_kbmap) don't use every
  parameter their signature requires; added (void) casts.
- unused-variable: proto_unix.c's handle_request() declares idx/str for
  two request cases that are themselves #ifdef USE_X11 - unused when
  built with --disable-x11.

Two warnings remain, both out of scope here: spnavd.c's SIGSEGV
fall-through (-Wimplicit-fallthrough) is addressed separately in FreeSpacenav#142,
and xdetect_linux.c's "ISO C forbids an empty source file" is a
pre-existing -Wpedantic warning (already present before this change,
independent of -Wextra) when built with --disable-x11.
@jtsiomb

jtsiomb commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Nothing is gained by adding a lot of empty casts just to assure the compiler that we know we didn't use some arguments, so I don't want to introduce the -Wextra option, or the changes that become necessary just to silence these extra useless warnings.

Having said that, if you found some actual issues by using -Wextra, then those are welcome. If you wish to do that, change your pull request to only fix the real bugs, explain each included fix, and don't include the -Wextra flag in your changes.

@Pace17881

Copy link
Copy Markdown
Author

Sorry for the noise — I went back through every individual warning this surfaced and checked the actual call sites/guards rather than just the types. None of them change behavior for any value that's actually reachable at runtime (all guarded by pre-existing != -1 checks, or already zero-initialized by C's rules regardless). So there's no real bug fix left to submit here. Closing.

@Pace17881 Pace17881 closed this Sep 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants