inifile: Fix non-reentrant Find() returning pointer to static buffer

[grandixximo]

Problem

IniFile::Find() returned a pointer to a static buffer, causing reentrancy bugs when multiple Find() calls were made before using the first result:

const char *a = inifile.Find("A", "SEC");  // Points to static buffer
const char *b = inifile.Find("B", "SEC");  // Overwrites the buffer!
// 'a' now points to corrupted data

The code had a comment acknowledging this issue:

// WTF, return a pointer to the middle of a local buffer?
// FIX: this is totally non-reentrant.
static char line[LINELEN + 2] = "";

Solution

Changed IniFile::Find() to return std::optional<std::string> instead of std::optional<const char*>. Each caller now owns their own copy of the result.

Before

const char *a = inifile.Find("A", "SEC");  // Pointer to shared buffer
const char *b = inifile.Find("B", "SEC");  // Buffer overwritten - 'a' invalid

After

auto a = inifile.Find("A", "SEC");  // Own copy as std::string
auto b = inifile.Find("B", "SEC");  // Separate copy - both valid
printf("%s\n", a->c_str());         // Still works!

Changes Made

Core API Changes

• inifile.hh: Changed Find() signature to return std::optional<std::string>
• inifile.cc: Removed static buffer, return std::string by value
• emcIniFile.hh: Updated wrapper class to match new return type

Caller Updates (20 files)

All callers updated to use the new API:

• Use s->c_str() when passing to C APIs requiring const char*
• Use *s == "value" instead of strcmp() for string comparisons
• Use s->find_first_of() instead of strchr() for character searches
• Use s->length() instead of strlen()
• Use s->empty() instead of checking s[0]

Migration Guide

String Output

// Before
if (auto s = inifile.Find("TAG", "SEC")) {
    printf("%s", *s);  // *s was const char*
}

// After
if (auto s = inifile.Find("TAG", "SEC")) {
    printf("%s", s->c_str());  // s is std::string, use .c_str()
}

String Comparison

// Before
if (auto s = inifile.Find("TAG", "SEC")) {
    if (strcmp(*s, "VALUE") == 0) { ... }
}

// After
if (auto s = inifile.Find("TAG", "SEC")) {
    if (*s == "VALUE") { ... }  // Cleaner!
}

Character Search

// Before
if (auto s = inifile.Find("COORDS", "TRAJ")) {
    if (strchr(*s, 'X')) { ... }
}

// After
if (auto s = inifile.Find("COORDS", "TRAJ")) {
    if (s->find_first_of("xX") != std::string::npos) { ... }
}

C API Compatibility

The legacy C function iniFind() maintains backward compatibility but still uses static storage:

extern "C" const char *iniFind(FILE *fp, const char *tag, const char *section)
{
    static std::string result_storage;  // Still non-reentrant!
    auto result = f.Find(tag, section);
    if (!result) return nullptr;
    result_storage = *result;
    return result_storage.c_str();
}

Important: C code calling iniFind() must still copy the result immediately if it needs to be preserved. This preserves the original C API behavior for backward compatibility.

Benefits

• Thread-safe for C++ callers: Each call gets its own string copy
• No buffer lifetime issues: Results remain valid regardless of subsequent calls
• Cleaner code: Can use C++ string operations instead of C string functions
• No functional changes: All updates are behavior-preserving
• Backward compatible: C API maintains existing behavior

Build Status

✅ Compiles successfully with no warnings or errors

[BsAtHome]

Why not fix the C API at the same time? It isn't used many places.

[grandixximo]

Fixed the C API as well, both C and C++ APIs should now be reentrant/thread-safe.

[BsAtHome]

To be pedantic...
You might want to add a [[deprecated]] to iniFind() if that works in c++ functions with C linkage. Otherwise I suggest to use __attribute__((deprecated)).

[grandixximo]

No problem, testing some stuff on the 9d branch, will add deprecated ASAP.

[grandixximo]

Done, I just forced pushed, so it stays in two relatively clean commits, attribute((deprecated)) more in line with the codebase. Devs should get a warning on compilation if they try to use it