MiNT (Atari) port#2334
Conversation
The NetSurf one is older, so we now use Vincent Rivière's GCC port directly.
They're definitely not 4 bytes on Atari.
|
damn there's no config.h … |
TODO: check which AES is used.
|
Thanks for working on this port — it is clearly a serious effort, and I appreciate the amount of research and implementation work that went into it. That said, I am not in favor of merging this PR in its current form. My main concern is not that MiNT is niche by itself, but that this adds a new long-term maintenance surface for a platform that the project is unlikely to have active maintainers, regular users, or any CI coverage for. In practice, that means:
This PR also does not stay fully isolated in platform-specific files. It modifies a fair amount of shared/common code to accommodate MiNT. That raises the maintenance cost for everyone, not just MiNT users. For a platform with no verification pipeline and uncertain long-term usage, that tradeoff seems hard to justify. I am also uncomfortable with some of the compatibility techniques used here, especially changes like:
Even if these choices are understandable for getting an initial port running, they are difficult to accept in core shared code because they reduce confidence in correctness and make the codebase harder to reason about. So my current view is:
I would be more open to reconsidering this if the port could be reworked to:
Without that, I think merging this would likely create dead code over time rather than sustainable platform support. |
|
I'll be busy the rest of the month, but I'll look into that. Some thing will be hard to do though, unless adding a Maybe some function should be moved to something the platform can override instead of the ifdeffery… Or platform-specific includes? It should be possible to add proper CI since the cross tools are packaged for Debian, never did that on github so it's an occasion I guess. Might need to make a few gigs of space here though… Might even get ARAnyM with a headless console 🤔 |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| ErrorProne | 10 medium |
🟢 Metrics 417 complexity · 10 duplication
Metric Results Complexity 417 Duplication 10
AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces Atari MiNT support but contains several critical bugs that prevent correct operation on the target platform. Most notably, the logic for identifying hardware via system cookies is inverted, and the package caching mechanism is fundamentally broken. Directory recursion is effectively disabled because the fallback for missing POSIX directory APIs always identifies entries as non-directories.
From a security and stability perspective, there is a memory safety violation in the string processing module that will cause data corruption on Big Endian systems. Additionally, the toolchain configuration contains hardcoded absolute paths that will break builds outside of specific environments. These issues must be addressed before the port can be considered functional.
About this PR
- The use of stubs for 'openat' and 'fstatat' without proper fallbacks (like 'stat') leads to systemic failures in directory processing. A more robust compatibility layer is needed.
- The Atari toolchain files contain hardcoded absolute paths which limit portability. These should be replaced with relative paths or environment variables.
Test suggestions
- Verify BIOS version and date detection on various Atari TOS/MiNT versions
- Verify Board model identification via the _MCH cookie across ST, STE, Falcon, and ARAnyM
- Verify CPU type and frequency detection for 68000, 68030, 68060 (CT60), and ColdFire
- Verify that ffTimeGetTick correctly converts _hz_200 ticks to milliseconds on MiNT
- Test application behavior when relative file operations (openat) are stubbed to fail
- Improve unit test coverage for complex logic in src/common/impl/FFPlatform_unix.c
- Improve unit test coverage for complex logic in src/common/impl/io_unix.c
- Improve unit test coverage for complex logic in src/common/impl/processing_linux.c
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify BIOS version and date detection on various Atari TOS/MiNT versions
2. Verify Board model identification via the _MCH cookie across ST, STE, Falcon, and ARAnyM
3. Verify CPU type and frequency detection for 68000, 68030, 68060 (CT60), and ColdFire
4. Test application behavior when relative file operations (openat) are stubbed to fail
5. Improve unit test coverage for complex logic in src/common/impl/FFPlatform_unix.c
6. Improve unit test coverage for complex logic in src/common/impl/io_unix.c
7. Improve unit test coverage for complex logic in src/common/impl/processing_linux.c
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
| #ifndef __MINT__ | ||
| if (fstatat(dfd, entry->d_name, &stbuf, 0) < 0) { | ||
| #endif | ||
| isDir = false; | ||
| #ifndef __MINT__ | ||
| } else { | ||
| isDir = S_ISDIR(stbuf.st_mode); | ||
| } | ||
| #endif | ||
| } | ||
| if (isDir) { |
There was a problem hiding this comment.
🔴 HIGH RISK
Recursive directory traversal is broken for MiNT. The variable isDir is never set to true because the fstatat and S_ISDIR checks are excluded. As this file is high-complexity and lacks coverage, this failure is high risk. Use stat() or lstat() as a fallback when fstatat is unavailable to allow directory recursion.
| { -1, NULL, NULL } | ||
| }; | ||
|
|
||
| if (Getcookie(COOKIE__MCH, &value)) { |
There was a problem hiding this comment.
🔴 HIGH RISK
The logic is inverted. Getcookie returns non-zero (1) on success. The current implementation will return an error message when the cookie is actually found.
Summary
This adds support for the Atari MiNT OS (not the GNU/Linux distro).
Changes
Screenshots
Checklist