SP: Attempt to fix GetString() calls with cvars in ICARUS

[ensiform]

Cvar_VariableStringBuffer() does not like unallocated buffers with strlen().

Uses a local variable and cache the result in a map<> that is then cleared upon shutdown.

[AntiAnti]

Any hope to get this approved?

[mrwonko]

Adding a cache feels weird. Caches usually make things more complicated. I'm not familiar enough with the code to tell if they're necessary at a quick glance, I need to study the code in detail first.

How do I reproduce the issue this fixes?

[AntiAnti]

Adding a cache feels weird. Caches usually make things more complicated. I'm not familiar enough with the code to tell if they're necessary at a quick glance, I need to study the code in detail first.

How do I reproduce the issue this fixes?

Read any cvar in script. For example: print ( $get( STRING, "cvar_g_char_model")$ ); Result: game crash.

There is another possible, simplier fix for this bug. It wouldn't work for two cvar requests in the same expression (for example, in the IF expression). Not critical, as for me. It requires only one change:

// Q3_Interface.cpp, CQuake3GameInterface::GetString, line 10571
if( strlen(name) > 5 &&
    Q_stricmpn(name, "cvar_", 5) )
{
    static char cvarbuf[MAX_STRING_CHARS];

    // save in static buffer rather then NULL pointer
    gi.Cvar_VariableStringBuffer(name+5, cvarbuf, sizeof(cvarbuf));
    *value = cvarbuf;
    return true;
}

I need to fix this bug for my mod, which is almost done now.

[ensiform]

Are you saying the cache code still crashes?

The point of the cache code is to not reuse one buffer and clobber over it in cases of multiple calls.

[mrwonko]

Are you saying the cache code still crashes?

No, just that caches are naturally complex and best avoided. But given the constraints, it's acceptable here, it doesn't prevent re-evaluation, it's just a form of memory management.

[mrwonko]

I've formally approved this, but since I also contributed code changes I'd prefer a second opinion before we merge

[bartrpe]

I've formally approved this, but since I also contributed code changes I'd prefer a second opinion before we merge

ok merg pls

[ensiform]

Go ahead