Skip to content

fix: address fopen() patch followups in main.c#351

Merged
spe-ciellt merged 1 commit intogerbv:developfrom
SourceParts:fix/fopen-followups
Mar 5, 2026
Merged

fix: address fopen() patch followups in main.c#351
spe-ciellt merged 1 commit intogerbv:developfrom
SourceParts:fix/fopen-followups

Conversation

@rampageservices
Copy link
Copy Markdown
Contributor

Addresses all four items from #349:

1. logToFileFilename now uses g_fopen()

The --log file was still opened with raw fopen(), which fails silently on Windows for non-ASCII paths even after the argv UTF-8 conversion from #318. Switched to g_fopen() which handles UTF-8 paths correctly on all platforms.

Also preserves errno and includes strerror() in the error message for better diagnostics.

2. #include <glib/gstdio.h> is now justified

The include was added in #318 but wasn't actually needed at the time. With the g_fopen() call above, it's now required.

3. #ifdef G_OS_WIN32#ifdef WIN32

Changed to match the existing style used consistently throughout main.c (bind_textdomain_codeset, screenRenderInfo.renderType, etc.). Both macros resolve identically on Windows.

4. Argv leak documented

Added a block comment explaining that the g_locale_to_utf8() allocations intentionally replace the C runtime's argv[i] pointers and are never freed — they must outlive main(). The block only compiles on Windows, so it won't appear in Linux Valgrind runs.

Closes #349

1. Replace raw fopen() with g_fopen() for --log file, so non-ASCII
   paths work on Windows after the argv UTF-8 conversion.

2. The #include <glib/gstdio.h> already present in main.c is now
   justified by the g_fopen() call above.

3. Change #ifdef G_OS_WIN32 to #ifdef WIN32 to match the existing
   style used throughout main.c.

4. Add a comment explaining the intentional argv pointer leak from
   the codepage conversion — the converted strings must outlive main()
   and this block only runs on Windows, invisible to Linux Valgrind.

5. Preserve errno from g_fopen() failure and include strerror() in
   the error message.

Closes gerbv#349
@spe-ciellt spe-ciellt added the pr-fixes Fixes that fixes fixes. Should refer to the original fix, not listed as fix in the release notes. label Mar 5, 2026
@spe-ciellt spe-ciellt merged commit fcaeab9 into gerbv:develop Mar 5, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-fixes Fixes that fixes fixes. Should refer to the original fix, not listed as fix in the release notes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Minor Issues Found in fopen() patch

2 participants