[steen-lund]

Resolves #228

Summary

This fixes the compiler warnings that GCC 16 reports across the program, dopus5.library and the modules. None of these changes is meant to change behaviour. I left out the fixes that do change behaviour, so they can be reviewed separately.

What's fixed

• Missing braces around initializer (-Wmissing-braces): command tables, Eliza string tables, TagItem arrays, flat initializer tables, 2D arrow image data
• Unused variables and functions (-Wunused-variable, -Wunused-but-set-variable): dead variables removed or marked UNUSED, and the non-Amiga-only SFTP helpers and timing variables guarded
• Discarded const qualifier (-Wdiscarded-qualifiers): the dopus5.library API made const-correct, plus the get() remote path, module data and module_info
• Incompatible pointer type (-Wincompatible-pointer-types): FTP update callbacks, ToolTypes, stpblk
• Char subscript or ctype argument (-Wchar-subscripts): isspace() and similar calls cast to unsigned char
• Misleading indentation (-Wmisleading-indentation): the ftp module, plus the nested ifs in the CPU detection braced (-Wdangling-else)
• Trigraphs (-Wtrigraphs): ftp debug output
• Old-style (K&R) function definitions: converted to prototypes
• Macro redefinition: the ID_* ILBM chunk IDs and AFF_68060 are guarded with #ifndef, because the NDK defines them
• Missing noreturn: exit() is marked as not returning in each libinit.c
• Address of array compared to NULL (-Waddress): only where the fix doesn't change behaviour (AmFTP profile strings, FTP site name, filetype name copy)

Things to review

• Library API: AddObjectList(), AddWindowMenus(), BuildMenuStrip(), SetConfigWindowLimits(), Att_NewNode(), WriteBuf() and NewConfigWindow.dims now take const pointers. This applies to every header flavour, including the OS4 interface, and to the OS4, MorphOS and AROS stubs. The library vectors and registers don't change, but it is a source change for module code: modules that wrote the window size through newwin.dims->... must set it in their own ConfigWindow instead, as the ftp module now does. The ChangeLog says so. The about module keeps its image data writable, because the AROS build byte-swaps it in place.
• Build flags: the warning flags are now passed to the linker too, because LTO generates the code at link time; without this, link-time warnings never show up. The code is now built with -fno-strict-aliasing, because it casts between Exec list structures throughout.
• Stale objects: the makefiles don't track header dependencies, so after switching to this branch run make os3 clean before building. Old objects that still use the previous GUI_Glue layout cause an LTO type-mismatch warning.

Not included

GCC also flags some code where fixing the warning changes what the code does:

• the AmFTP password decryption (encrypt() precedence)
• the icon author « » quotes
• the owner/group lister sort
• empty sound entries in configopus
• the FTP address-book message check

Testing

• Clean OS3 build (make os3 clean && make os3 all) with m68k-amigaos-gcc 16.2.0b: no errors. The remaining warnings are:

  • 13 warnings (12 -Waddress, 1 -Wparentheses) whose fixes would change behaviour
  • the deliberate #warning reminders
  • the two stack-frame warnings that master already has (config_open.c, XADopus.c)

• make -C source/Modules/ftp -f makefile.tests test passes.

• make -C source/Modules/ftp -f makefile.tests test-libssh2 passes.

• The full CI matrix passes on my fork. I compared its warnings with the master CI run for ba7da680, and no platform has a warning that master doesn't. One MorphOS -Wpointer-sign warning at config_environment.c:130 only changes its text from char * to const char *. Because eliza.c now includes dopus.h (for the shared strstri() prototype), the existing AROS/MorphOS header warnings about ID_PFS2_DISK, ID_PFS3_DISK and SimpleRequestTags being redefined appear once more. Warning counts:

  Job	master	this branch
  os3 debug / release	869 / 954	27 / 27
  os3-68000 debug / release	869 / 954	27 / 27
  os4 debug / release	2332 / 2352	1504 / 1522
  mos debug / release	2101 / 2135	1278 / 1310
  i386-aros debug / release	1836 / 1856	1028 / 1046
  x86_64-aros debug / release	1855 / 1877	1046 / 1066
  aarch64-aros debug / release	2066 / 2068	1225 / 1225

  The counts leave out #warning directives and lto-wrapper notes. The other platforms still have a lot of warnings; this PR covers what GCC 16 reports on OS3.

Housekeeping

• The ChangeLog has a [dopus5 next] entry. The version bump is deferred, following the recent entries.
• DOpus5.guide doesn't need changes, because nothing user-visible changes.

🤖 Generated with Claude Code

[midwan]

Thanks for this, it's a big cleanup and most of it checks out. I went through the whole diff; CI is green on all platforms, but there's one behaviour regression and one undocumented build-flag change that need addressing before merge.

Blocker

• FTP "Delete All" no longer suppresses further confirmations in lister_delete() (inline comment below).

Should fix

• -Wno-error=incompatible-pointer-types -Wno-error=implicit-function-declaration -Wno-error=implicit-int were added to the OS3 WARNINGS without mention in the PR description, ChangeLog or commit message (inline comment below).

Worth calling out

• NewConfigWindow.dims is now const in the public SDK header. The library vectors are fine (I checked SFD, FD, clib, defines, inline, ppcinline, the OS4 interface and the OS4/MOS stubs), but this is a source-compat break for third-party module code that writes through newwin.dims->..., exactly what ftp_addressbook.c had to be changed for. Please state that explicitly in the ChangeLog.
• "No behaviour change" isn't runtime-verified. -fno-strict-aliasing changes codegen for every OS3 binary, and testing covers builds and FTP host tests only. Please do at least one Amiberry run before merge: startup, a lister, FTP delete with confirmations, and the About window (its AROS image byte-swap already broke once on this branch, fixed in 694a111).

Nits

• about.c 324/363/532/557: the (UWORD *) casts on logo_image[...] and image->imagedata are no-ops (both are already non-const UWORD); leftovers from before 694a111. Only the (ULONG *)image->palette cast is needed.
• eliza.c:37 keeps a local strstri prototype; better to include misc.h so the two can't drift (that's the kind of thing that produces LTO type-mismatch warnings).
• lister_rename: removing abort leaves a stray blank line before the closing brace.
• defines/dopus5.h and ppcinline/dopus5.h: the macro continuation backslashes are no longer aligned; config_filetypes.h:66 has ;// without a space. A clang-format pass would sort these.
• The PR description lists GetPointerPkt, InputBase, SwapListNodes, IPC startup functions, the FEAT parser removal and the timeval init, but those are already on master via 17e1bfb and aren't in this diff. Worth trimming so the description matches the change.

Verified fine

• The function_data.c and FTP table rebraces keep order and terminators; the data.c arrow pairs keep all 22 words in order for both images.
• The CPU-detection braces match the old dangling-else binding.
• The lst_dos_err parentheses match existing precedence and the identical condition at line 865.
• The AmFTP *p->amftp_* checks are equivalent: the entry is MEMF_CLEAR.
• ftp_win writes are equivalent: newwin.dims = &ftp_win is unconditional.
• class_strings as const UWORD * matches matchtype_labels.
• The module_info casts are safe: commands.c only reads the result.

[midwan]

This removal changes behaviour. Unlike lister_findfile/lister_getsizes (where confirm_entry is always 1, so quiet really was dead), here quiet is read on the next loop iteration at lines 1775 and 1796 (if (!quiet && ...)). Return value 2 is the MSG_FTP_DELETEALL button, so with the assignment gone, pressing "Delete All" deletes that one entry and then prompts again for every remaining one.

Please restore it here:

					// Delete all?  Show no more confirmation requesters
					if (confirm_entry == 2)
						quiet = 1;

					if (confirm_entry == 1 || confirm_entry == 2)

[midwan]

The three -Wno-error= flags aren't mentioned in the PR description, ChangeLog or the 38b5eb5 commit message. They downgrade GCC 14+'s default errors back to warnings, and via LDFLAGS += $(WARNINGS) they now apply at LTO link time too.

The remaining-warnings list in the description has nothing in these categories, so they don't seem to be needed. Their only effect would be letting a future implicit declaration or pointer mismatch through as a warning, which on m68k with register-based library calls tends to become a runtime bug. Please drop them, or say which build fails without them.

[midwan]

ABI is unchanged, but this is a source-compat break for third-party modules that write through newwin.dims->... (as ftp_addressbook.c did). Fine with me if intentional, but please call it out explicitly in the ChangeLog as affecting module source.

[steen-lund]

🤖 This comment was generated by Claude Code and reviewed by @steen-lund.

Thanks for the thorough review, all points taken. Updated in place: each fix is folded into the commit that introduced the problem.

• Delete All: you're right, I treated the three quiet assignments the same, but in lister_delete() it's read on the next iteration. Restored.
• -Wno-error= flags: not needed; a clean OS3 build with GCC 16 gives the same result without them. Dropped. I also corrected that commit's message.
• NewConfigWindow.dims: intentional, and now called out in the ChangeLog as a source change for modules.
• Nits:
  • Removed the no-op (UWORD *) casts in about.c.
  • eliza.c now gets strstri() from dopus.h; misc.h on its own doesn't compile there. As a side effect, the existing AROS/MOS header redefinition warnings show up once more for that file.
  • Removed the stray blank line in lister_rename.
  • Realigned the backslashes in the macros this PR touches and fixed the ;//.
• Description and commit messages: I trimmed them to match the diff; those items landed on master in 17e1bfb0.
• Runtime: I'll do an Amiberry run (startup, a lister, FTP delete with confirmations, About) and report back here.

[steen-lund]

🤖 This comment was generated by Claude Code and reviewed by @steen-lund.

Runtime test on real hardware: Amiga 4000, OS3 release build (debug=no) of 6e7df4f8, built 24.10.2026.

• Startup: OK.
• Listers: OK, including an FTP lister.
• FTP delete with confirmations: OK. With multiple files and drawers selected, I pressed Proceed and then "Delete All" in the first per-entry requester. The rest were deleted with no further requesters. The first "OK to delete" requester's "Delete All" also works.
• About window: opens and draws the logo correctly. The AROS byte swap isn't exercised on OS3; on AROS it is only covered by the CI builds.

[midwan]

Thanks, all the points from the last round are sorted. I checked each one against 6e7df4f8.

One thing left. source/Library/dopusbase.h has its own copy of NewConfigWindow, and there dims is still ConfigWindow *. The library builds against that copy, which is why L_OpenConfigWindow() can pass newwindow->dims to L_CalcWindowDims(ConfigWindow *) without a warning, while modules see the const version from libraries/dopus5.h. Could you:

• make dims const ConfigWindow * in dopusbase.h too, and
• make the second argument of CalcWindowDims() const ConfigWindow * in dopuslib.h, layout_routines.c, the SFD, clib, defines, inline, ppcinline, the OS4 interface and the OS4/MorphOS stubs. It only reads win_pos, so the vector and registers don't change. Please add it to the list of const API calls in the ChangeLog entry.

The branch also conflicts with master in ChangeLog now (#232 and #233 added [dopus5 next] entries). After a rebase, please keep all three entries with yours on top.

I built the merged tree with GCC 6.5 (the CI image) and GCC 16.2.1b. Both build with no errors, and the GCC 16 warnings match the list in the description.

[steen-lund]

🤖 This comment was generated by Claude Code and reviewed by @steen-lund.

Done, rebased onto ba7da680:

• dims in dopusbase.h's NewConfigWindow is now const ConfigWindow *.
• CalcWindowDims() takes a const ConfigWindow * in dopuslib.h, layout_routines.c, the SFD, clib, defines, inline, ppcinline, the OS4 interface and 68k vector, and the OS4/MorphOS stubs. Both are folded into the const-correctness commit, and CalcWindowDims() is in the ChangeLog's list of const API calls.
• The ChangeLog conflict is resolved, with all three [dopus5 next] entries kept and ours on top.

A clean OS3 build with GCC 16 has the same warnings as before. CI on my fork is green on all platforms, and compared with master's run for ba7da680 no platform gains a warning.

[midwan]

Thanks!