Lua FFI fixes - #657
Open
CoffeeFlux wants to merge 12 commits into
Open
Conversation
clipboard_get returns strndup'd memory but typed it const char *, and aegisub.internal.ffi's string helper deliberately only frees non-const char * (const means the native side retains ownership). Type it char * so the buffer is actually freed.
regex_compile strdups the exception text into the error out-param, but the moon side read it with ffi.string, which does not free. Use ffi_util.string, which does.
CoffeeFlux
force-pushed
the
lua-ffi-fixes
branch
from
August 4, 2026 17:59
8df1e1b to
e998677
Compare
boost::regex can throw -- most notably on invalid UTF-8 via the UTF-8-to-UTF-32 iterators (the crash backtrace in Aegisub#99), and on regex complexity/backtracking limits -- but only regex_compile caught anything. An exception unwinding through the LuaJIT interpreter or JIT frames which called these functions is undefined behavior and can take down the process. Catch exceptions in regex_search/regex_match/regex_replace and report them through a char **err out-param as lfs already does; the moon side raises them as proper Lua errors.
On a bogus conversion the wrapper set the error out-param but then ran the conversion anyway and returned a result, which the moon side's error branch never freed. Return null once the error is set; the moon side already handles a null result plus an error message correctly.
Previously OOM meant memcpy through a null pointer. The Lua side already maps a null return to nil.
The impl functions return a C bool, which LuaJIT's ffi converts to a Lua boolean, and tonumber(true) is nil -- so these four functions returned nil, nil on success, indistinguishable from failure to any caller checking the standard lfs true-or-nil,err convention. Found because busted's own chdir call refuses to run the automation test suite against the aegisub lfs.
error 2, err raises the number 2 with err as the level; failing to open a directory should raise the error message.
It was only used to generate unique names under /tmp, which os.tmpname does without an external rock (whose API has also drifted: uuid 1.0 refuses to run until an rng is explicitly configured). os.tmpname creates the file on POSIX, so unlink it and use just the name.
Builds the standalone Lua host from automation/tests and registers the busted suite as a meson test when busted is installed through luarocks for Lua 5.1; nothing has been running these tests since Travis went away. The suite's fixtures are POSIX-only, so it is only registered on non-Windows hosts. Run with: meson test --suite automation
This has been institutional knowledge since the /MT era of the old Visual Studio build broke automation (2016-2018); write it down where the next person flipping b_vscrt will find it.
CoffeeFlux
force-pushed
the
lua-ffi-fixes
branch
from
August 4, 2026 18:11
e998677 to
c882547
Compare
The test step only ran "gtest main" by name, so luajit-52 (and now the automation suite) never ran in CI. Install busted through luarocks on Ubuntu and macOS so the automation tests are registered; Windows keeps skipping them since luarocks has no painless setup there and the meson gate handles the absence.
CoffeeFlux
force-pushed
the
lua-ffi-fixes
branch
2 times, most recently
from
August 4, 2026 18:17
b3f261d to
f49d812
Compare
lfs.dir had no tests at all; the re specs pin that invalid UTF-8 is reported as a Lua error, which is the path that previously crashed (Aegisub#99); the unicode spec pins ICU's substitution behavior for invalid input. Notably lfs.dir's error branch turns out to be unreachable: DirectoryIterator swallows construction failures on every platform and yields an empty iteration instead.
CoffeeFlux
marked this pull request as ready for review
August 4, 2026 21:49
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.