# Static analysis of the A320E patch

**Subject:** `backend/genesys/`, tree = upstream `fcaa30a7` +
`a320e-genesys.patch` (17 files, +3394 / −28)
**Date:** 2026-09-03 for cppcheck and the style gate, which were re-run
against this version; **2026-08-29 for the clang-tidy counts**, which were
not — the tree has grown by 3 files and about 620 lines since, all of it in
the motor path. Each row below says which is which, because a stale number
presented as current is worse than an absent one.
**Tools:** cppcheck; clang-tidy (LLVM 22.1.8); upstream's own
`tools/style-check.sh`

Ralph asked for careful review, machine review included. This document is that
review. It is meant to be an argument you can check, not an attachment, so the
method is stated first and every warning left unaddressed is listed with the
reason.

## Summary

| Tool | Findings in total | **On lines this patch adds** | Genuine defects in the added code |
|---|---|---|---|
| cppcheck (re-run 2026-09-03) | 18 lines (17 distinct defects) | **0** | **0** |
| clang-tidy, `gl124.cpp` + `genesys.cpp` (2026-08-29 tree) | 3 186 unique warnings | 247 | **0** (all reviewed, see §3) |
| clang-tidy, the same plus `low.cpp` (2026-08-29 tree) | 3 656 unique warnings | 268 | **0** (the 21 in `low.cpp` are listed in §3) |
| upstream `tools/style-check.sh` (re-run 2026-09-03) | — | **17/17 files pass** | — |

The added code passes cppcheck and upstream's style gate cleanly. All 18
cppcheck findings are in upstream's own code; 3 of those are genuine defects
and are described in §2 in case they are of use to you. The 247 clang-tidy
warnings on added lines are style rules that conflict with the genesys
backend's own conventions; following them would make the patch worse rather
than better (§3).

`low.cpp` is analysed on a second line of its own because the patch grew into
it (the x scale correction, the shading width and the calibration-cache guard
all live there) after this document's first version, and the historical
numbers above are only comparable over a fixed set of files. Its 21 warnings
are of the same kind as the rest: trailing return types, magic numbers, `f`
suffixes, include-cleaner. The one that reads like a defect,
`init-variables` at the cache guard, is a false positive — the variable is
initialised in its declaration, whose initialiser spans two lines.

## 1. Method: findings are classified per line, not per file

A finding counts as ours only if it lands on a line the patch actually adds.
The file-level rule — "the file appears in the patch, so the defect is ours" —
gives a badly wrong answer here, and it is worth saying why, because it is an
easy mistake to make when reviewing a patch of this shape.

`enums.h` is in the patch, but what the patch adds to it is **five enum
values** (`PLUSTEK_OPTICPRO_A320E` in five lists). It does not touch any of the
serialisation functions cppcheck complains about. The same holds for
`genesys.cpp`: the findings at lines 5872, 5888 and 5945 are in upstream's
`attach_one_device`, `config_attach_genesys` and `read_calibration`, none of
which the patch modifies.

File-level classification gives 10/18 as "ours" (56 %). Line-level gives
**0/18**. The classifier compares each finding's line number against the line
numbers the patch's hunks add, and it is run against a clean upstream+patch
tree rebuilt from scratch on every run, so the state of any working tree cannot
affect the result.

## 2. cppcheck — 17 defects, all of them upstream's

```
cppcheck --std=c++17 --suppress=missingInclude --suppress=missingIncludeSystem -q backend/genesys/
```

| Location | Check | Class | Reasoning |
|---|---|---|---|
| `motor.cpp:59` | uninitvar + uninitStructMember | **GENUINE** | `motor.h:92` declares `unsigned max_step_count;` with no initialiser while its sibling fields are `= 0`, and `MotorSlope::create_from_steps` does not set it. |
| `usb_device.cpp:33` | throwInNoexceptFunction | **GENUINE** | `~UsbDevice()` calls `close()`, which can throw. The destructor is implicitly `noexcept`, so this is `std::terminate`. |
| `test_usb_device.cpp:40` | throwInNoexceptFunction | **GENUINE** | The same pattern in `~TestUsbDevice()`. |
| `serialize.h:86,115` | uninitvar | false positive | `serialize(std::istream&, T&)` **writes** the variable (`serialize.h:53`, `str >> x`). It is the read direction, not a read of an uninitialised value. |
| `enums.h:51,85,129,210,274,328,609` | uninitvar | false positive | The same deserialisation pattern (`unsigned value; serialize(str, value);`). |
| `sensor.h:106` | uninitvar | false positive | The same pattern. |
| `genesys.cpp:5945` | uninitvar | false positive | `size_t version; serialize(str, version);` — the read direction again. |
| `genesys.cpp:5872` | throwInNoexceptFunction | false positive | `wrap_exceptions_to_status_code` (`error.h:131`) ends in `catch (...)`. Nothing escapes. |
| `image_pipeline.cpp:822` | throwInNoexceptFunction | false positive | `catch_all_exceptions` (`error.h:174`) ends in `catch (...)`. |
| `genesys.cpp:5888` | throwInNoexceptFunction | uncertain | `sanei_usb_attach_matching_devices` is sane's C interface; the callback `attach_one_device` is itself `noexcept` and wraps its exceptions. It does not throw in practice, but cppcheck cannot prove that from the genesys sources alone. |

The false positives come from how the tool is run: without include paths
(`--suppress=missingInclude`) cppcheck cannot resolve the `serialize`
overloads and cannot see the `catch (...)` arms inside the template wrappers.
Supplying include paths removes them at a large cost in run time; the result is
serviceable once the classification has been done by hand.

**The three genuine defects are upstream's, not this patch's.** They are listed
here because you may want them, not as anything expected of you in return.

## 3. clang-tidy — 247 warnings on added lines, no defects

```
clang-tidy --checks='-*,modernize-*,cppcoreguidelines-*,misc-*,readability-*' gl124.cpp genesys.cpp
```

| Check | On added lines | Action |
|---|---|---|
| `avoid-magic-numbers` / `readability-magic-numbers` | 71 | **No** — the numbers in a register driver are the device's register values. Upstream's genesys is full of them; named constants would make these lines stylistically foreign to the file they live in. |
| `readability-identifier-length` | 40 | **No** — `x`, `c`, `i` are genesys's own convention in loops. |
| `readability-math-missing-parentheses` | 22 | **No** — checked: the order of evaluation is the intended one (e.g. the parity filter's formula `g[x] - sum_own/n_own + sum_all/n_all`). |
| `misc-const-correctness` | 20 | Could be done. Small, local, does not change the character of the diff. |
| `readability-uppercase-literal-suffix` | 16 | **No** — `1.5f` is how the whole of genesys writes it. |
| `misc-include-cleaner` | 15 | **No** — unreliable without a compilation database; upstream's own code has 147 of the same. |
| `misc-use-anonymous-namespace` | 10 | **No** — would require changing upstream's file structure. |
| `modernize-use-trailing-return-type` | 8 | **No** — genesys does not write them; upstream's own code has 503 of the same. |
| `readability-braces-around-statements` | 8 | **No** — a single-line `if (x < 0) x = 0;` is genesys's habit. |
| `cppcoreguidelines-narrowing-conversions` | 7 | **No** — checked: `int` to `float` in the calibration arithmetic is deliberate. |
| `readability-use-std-min-max` | 5 | **No** — the `if` form reads better where the bound is a register's range. |
| `non-private-member-variables-in-classes` | 4 + 2 | **No** — upstream's class structure. |
| `readability-isolate-declaration` | 4 | **No** — style. |
| `readability-implicit-bool-conversion` | 2 | **No** — `sensor.shading_factor ? … : 1` is genesys's own zero-guard. |
| `cppcoreguidelines-pro-bounds-pointer-arithmetic` | 2 | **No** — `memcpy` into a raw buffer, bounds checked in the code. |
| singles (`function-cognitive-complexity`, `init-variables`, `unused-parameters`, `avoid-c-arrays`, `avoid-c-style-cast`, `pro-bounds-constant-array-index`) | 6 | Each checked individually, below. |

**What is not covered by the clang-tidy count.** The 2026-09-02…03 motor work
(the measured acceleration curve, the per-phase `FWDSTEP`/`BWDSTEP`, `FSTPSEL`,
the full-length slope tables) added about 620 lines after that run. cppcheck
and the style gate were re-run over them and are clean; clang-tidy was not
re-run. The added code is of the same kind as what §3 already covers — table
data, register writes and comments — so I do not expect the character of the
findings to change, but I have not measured that and am not claiming it.

**Why the count rose from 236 to 247 since 2026-08-28.** Same clang-tidy
(LLVM 22.1.8), same command, same file set — the code changed. The patch grew
from +2377 to +2773 lines: the x scale correction and its origin rounding, the
widened shading window, and two diagnostics (the shading source and the white
reference running out). Eleven more warnings for 361 more lines is a much lower
rate than the previous round, and for the same reason — the new code is
control flow and comments rather than dense calibration arithmetic.

**And why it rose from 157 to 236 before that (2026-08-25 to 08-28).** The
patch grew from +1787 to +2377 lines (parity sign normalisation, black level
per parity group, white level compensation). That is 33 % more lines and 50 %
more warnings,
which matches the character of the new code: it is dense calibration
arithmetic, and `magic-numbers` (55 → 70) and `identifier-length` (23 → 39)
carry half the increase between them.

The check that this is a change in the code and not in the tool: **upstream's
own numbers did not move** — magic-numbers 584 → 583, trailing-return-type
503 → 503, non-private-member 211 → 211, identifier-length 207 → 207,
nodiscard 181 → 181.

**Individually checked cases:**

- `genesys.cpp:2357`, "variable 'gain_pixels' is not initialized" — **false
  positive.** The variable is initialised in the same statement by a
  multi-line `static_cast` expression; without a compilation database
  clang-tidy does not parse it.
- `genesys.cpp:1749`, `a320e_offset_calibration` has cognitive complexity 37
  (threshold 25) — **not a defect, but recorded honestly.** It is a search
  loop over three channels with parity groups and its own stopping
  conditions. Upstream's genesys has functions in the same class. Splitting it
  is possible but would scatter the search logic across three places where it
  reads worse than it does now.
- `gl124.cpp:506`, "parameter 'sensor' is unused" — **deliberate, and argued
  in the code.** `gl124_set_a320e_fe` looks the sensor up by
  `dev->settings.xres` rather than using the calibration sensor it is passed;
  the comment at lines 562-566 says why (during an 800 dpi scan `set_fe` runs
  14 times, and in six of them the sensor passed in is the 100 dpi row). The
  parameter is the shape of the `CommandSetGl124::set_fe` interface and cannot
  be dropped.
- `gl124.cpp:1746,1899`, implicit bool conversions — **not defects.**
  `sensor.shading_factor ? sensor.shading_factor : 1` is genesys's own way of
  writing a zero guard.
- `gl124.cpp:1760,1761`, C-style cast and pointer arithmetic — **not defects.**
  A `memcpy` into a raw buffer; the bounds are checked on the preceding line
  (`if (src + 4 <= (std::uint32_t)size)`).
- `gl124.cpp:2233`, `gpios[idx]` — **not a defect.** `idx` is looked up from
  the table in the same function; upstream's own pattern.
- `gl124.cpp:357`, C array — **not a defect.** A static initialiser list for a
  register table, genesys's way.
- `gl124.cpp:1741`, the parenthesisation warning — **not a defect.** `sum_own`
  is a `float`, so `sum_own / n_own` is floating-point division, and the
  `-` / `+` order is the one intended.
- `gl124.cpp:1730,1741`, the narrowing warnings — **not defects.** Deliberate
  conversions from floating-point means to register values, clamped to
  `0…65535`.

**Limitation, stated plainly:** the run was made without a compilation
database, so the result is partial and contains false positives. A full run
would need `compile_commands.json` (`bear -- make`). It is not worth building:
2 939 of the 3 186 warnings are in upstream's code (most common:
magic-numbers 583, trailing-return-type 503, non-private-member 211,
identifier-length 207, nodiscard 181), and upstream is evidently not following
the C++ Core Guidelines — genesys's own style is a different one.

### `low.cpp`, analysed separately

The patch reached `low.cpp` on 2026-08-29 (`a320e_x_scale`,
`a320e_shading_width_mm`, the calibration-cache guard). Running the same
command with that file added gives **21 warnings on added lines**, all of the
same classes as above:

| Check | Count | Action |
|---|---|---|
| `modernize-use-trailing-return-type` | 5 | **No** — genesys writes leading return types throughout. |
| `avoid-magic-numbers` / `readability-magic-numbers` | 3 | **No** — 1.0075 and 330.2 are measured device constants and each is commented with the measurement that produced it. |
| `readability-uppercase-literal-suffix` | 4 | **No** — as above. |
| `misc-include-cleaner` | 4 | **No** — no compilation database. |
| `misc-const-correctness` | 2 | Could be done; local. |
| `cppcoreguidelines-init-variables` | 1 | **False positive** — the variable is initialised in its declaration, whose initialiser spans two lines. |
| `cppcoreguidelines-pro-type-vararg` | 1 | **No** — it is the `DBG` macro, upstream's own logging. |
| `readability-identifier-length` | 1 | **No** — `ms` in the homing timeout. |

## 4. clang-format was deliberately not used

This matters for reviewing the patch, so it is worth being explicit about.

- Upstream **ships no `.clang-format` file.** Without one, clang-format falls
  back to the LLVM style: 2-space indent, `Type *x`, brace on the function's
  line. Genesys's own style is 4 spaces, `Type& x`, brace on its own line.
  Running it does not tidy the code, it **swaps the style for a different
  one**.
- This was learned the hard way. On 2026-08-23 a `clang-format -i` over
  `backend/genesys` produced **83 files, +33 233 / −31 331 lines**, including
  files this work does not touch at all (gl646, gl841, gl843, gl846,
  `pixma/scripts/*.py`, `tools/*.sh`, `autogen.sh`). The patch would have been
  33 000 lines of formatting noise around 2 000 lines of substance. It was
  reverted, and a guard was added so it cannot happen again unnoticed.
- **Upstream's actual style gate is `tools/style-check.sh`**: UTF-8, no
  trailing whitespace, newline at end of file, no blank lines at the end.
  All 17 files touched by this patch pass it.

## 5. Reproducing this

From a tree with the patch applied:

```sh
cppcheck --std=c++17 --suppress=missingInclude \
         --suppress=missingIncludeSystem -q backend/genesys/
clang-tidy --checks='-*,modernize-*,cppcoreguidelines-*,misc-*,readability-*' \
           backend/genesys/gl124.cpp backend/genesys/genesys.cpp
clang-tidy --checks='-*,modernize-*,cppcoreguidelines-*,misc-*,readability-*' \
           backend/genesys/gl124.cpp backend/genesys/genesys.cpp backend/genesys/low.cpp
tools/style-check.sh
```

To classify findings the way §1 describes, compare each finding's line number
against the line numbers added by the patch's hunks; a file-level comparison
will mislead you.