Re: Patch for lua-5.2.0-beta-rc2

[Patrick Donnelly <batrick@...>]

On Fri, Jun 24, 2011 at 11:57 AM, François Perrad
<francois.perrad@gadz.org> wrote:
> 2011/6/24 Roberto Ierusalimschy <roberto@inf.puc-rio.br>:
>>> a minor improvement/optimization of setpath().
>>
>>     /* replace ";;" by ";AUXMARK;" and then AUXMARK by default path */
>>     path = luaL_gsub(L, path, LUA_PATH_SEP LUA_PATH_SEP,
>>                               LUA_PATH_SEP AUXMARK LUA_PATH_SEP);
>> +    lua_remove(L, -1);
>>     luaL_gsub(L, path, AUXMARK, def);
>> -    lua_remove(L, -2);
>>   }
>>
>> This "improvement" seems to have a subtle bug. With the first remove,
>> the string 'path' may not be anchored in Lua anymore. If the next gsub
>> triggers some garbage-collection cycles, 'path' may be collected while
>> still being used by gsub.
>
> setpath is a static function called only by luaopen_package,
> and the GC is stopped during the initialization of std libraries (in lua.c).
> But an embedder could call luaL_openlibs without this precaution.

This isn't an improvement or an optimization. 'path' must exist on the
stack before calling luaL_gsub to be correct. Your patch is wrong.

-- 
- Patrick Donnelly