Skip to content

Commit d0b92de

Browse files
committed
Move recurring bug patterns to copilot-instructions, keep skill workflow-only
The hardware-deploy skill should only cover deploy/debug workflow. General code-review pitfalls (input validation, NULL-sentinel APIs, SD file handle lifecycle) apply to any code review in this repo, not just hardware deploy sessions, so they belong in the always-loaded copilot-instructions.md instead, generalized as patterns rather than pointers to specific fixed bugs.
1 parent d681a56 commit d0b92de

2 files changed

Lines changed: 6 additions & 5 deletions

File tree

‎.github/copilot-instructions.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,11 @@ Do not invoke libdragon tools directly unless debugging the build system. Prefer
219219
- Keep behavior identical unless the bug itself requires behavior change.
220220
- After edits, validate touched files and summarize exactly which findings were fixed.
221221

222+
### Recurring pitfall patterns (watch for these on any review)
223+
- **Unchecked ints from user-editable config**: any value parsed from a `.ini`/config file (e.g. `rom_offset`) must be range-checked before use in pointer/size arithmetic like `SIZE - value`, which silently underflows on out-of-range input.
224+
- **NULL-sentinel API arguments**: avoid a single parameter meaning two different things depending on whether it's NULL (e.g. "use this new value" vs "reuse whatever was last set"). Split into a setter + a no-arg action function instead.
225+
- **Multiple SD card file handles open at once**: closing/reopening one SD-backed resource (e.g. audio player track) while another SD file handle from a different subsystem (e.g. custom BGM) is still open can corrupt the second open. Always close the first subsystem's handle before initializing/reopening another SD-backed resource.
226+
222227
### Minimal Change Policy (Strict)
223228
- Be laser-focused on the exact user request; do not broaden scope.
224229
- Do not refactor unless the user explicitly asks for refactoring.

‎.github/skills/n64flashcartmenu-hardware-deploy/SKILL.md‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,4 @@ docker exec <container> bash -c "sc64deployer --remote host.docker.internal:9064
9898

9999
If this fails with `Couldn't init the SD card: SD card is locked by the N64 side`, the console is currently powered on and has the SD card mounted — **ask the user to power off the console**, then retry. `sc64deployer reset` alone is not sufficient; the console must actually be off.
100100

101-
## Known code gotchas (src/menu)
102-
103-
- `sound.c`/`sound.h`: BGM/SFX subsystem. Avoid NULL-sentinel-argument APIs (e.g. `sound_init_bgm(char*)` meaning "new path" vs "reuse last path" depending on NULL) — split into a setter (`sound_set_bgm_path`) + no-arg action (`sound_init_bgm(void)`) instead.
104-
- `views/music_player.c` `deinit()`: must call `audioplayer_deinit()` (closes the SD file handle of the currently playing track) **before** `sound_init_default()` (which may reopen a custom SD-based BGM file) — two SD files open at once corrupts the second `wav64_t`, tripping the `wave->channels == 1 || wave->channels == 2` assertion in `mixer_ch_play`.
105-
- `cart_load.c`: any numeric value parsed from a user-editable `.ini` file (e.g. `rom_offset`) must be bounds-checked before use in pointer/size arithmetic like `MiB(64) - rom_offset`, which silently underflows on an out-of-range input.
101+
For general code-review pitfalls (input validation, resource lifecycle, etc.), see `.github/copilot-instructions.md`.

0 commit comments

Comments
 (0)