View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0000870 | FSSCP | scripting | public | 2006-03-22 20:04 | 2007-01-21 09:15 |
| Reporter | Zephiris | Assigned To | WMCoolmon | ||
| Priority | normal | Severity | crash | Reproducibility | always |
| Status | closed | Resolution | fixed | ||
| Summary | 0000870: VS2005 and Lua | ||||
| Description | Your LUA initialization code depends largely on features in VS2005 (and that probably shouldn't be used in C++ anyway), namely a pointer past the end of an array, et cetera. Because of poor memory management or checking on several OSes (this would probably also fail on FreeBSD), this slips through. I solved it by using C++ vector iterators with 'begin' and 'end', which should equal the '0' and 'last element+1' which was being used before. With VS2005 (and version 8 runtime) unfortunately, this is reported at runtime as a C runtime error, rather than a compile-time error. | ||||
| Additional Information | This mostly affects VS2005, but should be a general cleanup for everything else. | ||||
| Tags | No tags attached. | ||||
|
2006-03-22 20:04
|
lua.patch (11,886 bytes)
Index: code/parse/lua.cpp
===================================================================
RCS file: /home/fs2source/cvsroot/fs2_open/code/parse/lua.cpp,v
retrieving revision 2.44
diff -u -r2.44 lua.cpp
--- code/parse/lua.cpp 27 Feb 2006 01:21:03 -0000 2.44
+++ code/parse/lua.cpp 22 Mar 2006 20:51:22 -0000
@@ -4285,10 +4285,10 @@
// *************************Housekeeping*************************
-void lua_add_vars(lua_State *L, int table_loc, lua_lib_h *lib, lua_var_hh *var, lua_var_hh *var_end)
+void lua_add_vars(lua_State *L, int table_loc, lua_lib_i lib, lua_var_i var_begin, lua_var_i var_end)
{
//We have variables
- if(var != var_end || lib->Functions.size())
+ if(var_begin != var_end || lib->Functions.size())
{
//Set __index to special handler
lua_pushstring(L, "__index");
@@ -4306,7 +4306,7 @@
std::string str;
//Add variables
- for(; var < var_end; var++)
+ for(lua_var_i var = var_begin; var < var_end; var++)
{
//Set a bogus value here so index_handler knows it's there
//and not a typo
@@ -4379,7 +4379,7 @@
if(lib->Indexer != NULL)
{
//If we are using the index handler, put it in its special spot
- if(var != var_end || lib->Functions.size())
+ if(var_begin != var_end || lib->Functions.size())
{
lua_pushstring(L, "__indexer");
lua_pushboolean(L, 0); //Default is get
@@ -4414,6 +4414,9 @@
lua_State *L = lua_open(); /* opens Lua */
mprintf(("LUA: Initializing base Lua libraries...\n"));
+#if LUA_VERSION_NUM >= 501
+ luaL_openlibs(L);
+#else
luaopen_base(L); /* opens the basic library */
luaopen_table(L); /* opens the table library */
@@ -4422,6 +4425,7 @@
luaopen_string(L); /* opens the string lib. */
luaopen_math(L); /* opens the math lib. */
+#endif
if(L == NULL)
{
@@ -4429,16 +4433,16 @@
return 0;
}
- lua_lib_h *lib = &lua_Libraries[0];
- lua_lib_h *lib_end = &lua_Libraries[lua_Libraries.size()];
- lua_lib_h *obj = &lua_Objects[0];
- lua_lib_h *obj_end = &lua_Objects[lua_Objects.size()];
- lua_lib_h *libobj = NULL;
- lua_lib_h *libobj_end = &lua_Libraries[lua_Libraries.size()];
- lua_func_hh *func;
- lua_func_hh *func_end;
- lua_var_hh *var;
- lua_var_hh *var_end;
+ lua_lib_i lib_begin = lua_Libraries.begin();
+ lua_lib_i lib_end = lua_Libraries.end();
+ lua_lib_i obj_begin = lua_Objects.begin();
+ lua_lib_i obj_end = lua_Objects.end();
+ lua_lib_i libobj_begin = lua_Libraries.begin();
+ lua_lib_i libobj_end = lua_Libraries.end();
+ lua_func_i func_begin;
+ lua_func_i func_end;
+ lua_var_i var_begin;
+ lua_var_i var_end;
int i; //used later
//*****CHECK FOR BAD THINGS
@@ -4552,9 +4556,9 @@
//*****INITIALIZE ALL LIBRARY FUNCTIONS
mprintf(("LUA: Initializing library functions...\n"));
int table_loc;
- lib = &lua_Libraries[0];
- lib_end = &lua_Libraries[lua_Libraries.size()];
- for(; lib < lib_end; lib++)
+ lib_begin = lua_Libraries.begin();
+ lib_end = lua_Libraries.end();
+ for(lua_lib_i lib = lib_begin; lib != lib_end; lib++)
{
//If a library name is given, register functions as library items
//If not, register functions as globals
@@ -4588,9 +4592,9 @@
table_loc = lua_gettop(L);
- func = &lib->Functions[0];
- func_end = &lib->Functions[lib->Functions.size()];
- for(; func < func_end; func++)
+ func_begin = lib->Functions.begin();
+ func_end = lib->Functions.end();
+ for(lua_func_i func = func_begin; func != func_end; func++)
{
Assert(func->Name != NULL && strlen(func->Name));
if(func->Function == NULL)
@@ -4603,14 +4607,14 @@
lua_settable(L, -3); //Add it into the current lib table
}
- lua_add_vars(L, table_loc, lib, &lib->Variables[0], &lib->Variables[lib->Variables.size()]);
+ lua_add_vars(L, table_loc, lib, lib->Variables.begin(), lib->Variables.end());
}
else
{
//Iterate through the function list
- func = &lib->Functions[0];
- func_end = &lib->Functions[lib->Functions.size()];
- for(; func < func_end; func++)
+ func_begin = lib->Functions.begin();
+ func_end = lib->Functions.end();
+ for(lua_func_i func = func_begin; func != func_end; func++)
{
//Sanity checking
Assert(func->Name != NULL && strlen(func->Name));
@@ -4624,7 +4628,7 @@
lua_settable(L, LUA_GLOBALSINDEX);
}
- lua_add_vars(L, LUA_GLOBALSINDEX, lib, &lib->Variables[0], &lib->Variables[lib->Variables.size()]);
+ lua_add_vars(L, LUA_GLOBALSINDEX, lib, lib->Variables.begin(), lib->Variables.end());
}
//Handle objects and their methods in a library
@@ -4632,10 +4636,10 @@
//*****INITIALIZE OBJECT FUNCTIONS
mprintf(("LUA: Initializing object functions...\n"));
- lib = &lua_Objects[0];
- lib_end = &lua_Objects[lua_Objects.size()];
+ lib_begin = lua_Objects.begin();
+ lib_end = lua_Objects.end();
std::string str;
- for(; lib < lib_end; lib++)
+ for(lua_lib_i lib = lib_begin; lib != lib_end; lib++)
{
if(!luaL_newmetatable(L, lib->Name))
{
@@ -4654,7 +4658,7 @@
bool concat_oper_already = false;
//WMC - This is a bit odd. Basically, to handle derivatives, I have a double-loop set up.
- lua_lib_h *clib = lib;
+ lua_lib_h *clib = &(*lib);
for(i = 0; i < 2; i++)
{
if(i==0)
@@ -4666,15 +4670,15 @@
}
else
{
- clib = lib;
+ clib = &(*lib);
}
- func = &clib->Functions[0];
- func_end = &clib->Functions[clib->Functions.size()];
- var = &clib->Variables[0];
- var_end = &clib->Variables[clib->Variables.size()];
+ func_begin = clib->Functions.begin();
+ func_end = clib->Functions.end();
+ var_begin = clib->Variables.begin();
+ var_end = clib->Variables.end();
- for(; func < func_end; func++)
+ for(lua_func_i func = func_begin; func != func_end; func++)
{
Assert(func->Name != NULL && strlen(func->Name));
if(func->Function == NULL)
@@ -4717,7 +4721,7 @@
}
}
- lua_add_vars(L, table_loc, lib, var, var_end);
+ lua_add_vars(L, table_loc, lib, var_begin, var_end);
}
//Add concat operator if necessary
@@ -4736,8 +4740,8 @@
void output_lib_meta(FILE *fp, lua_lib_h *main_lib, lua_lib_h *lib_deriv)
{
- lua_func_hh *func, *func_end;
- lua_var_hh *var, *var_end;
+ lua_func_i func_begin, func_end;
+ lua_var_i var_begin, var_end;
int i,j;
bool draw_header;
fputs("<dd><dl>", fp);
@@ -4754,9 +4758,9 @@
break;
}
- func = &lib->Functions[0];
- func_end = &lib->Functions[lib->Functions.size()];
- for(; func < func_end; func++)
+ func_begin = lib->Functions.begin();
+ func_end = lib->Functions.end();
+ for(lua_func_i func = func_begin; func != func_end; func++)
{
if(!strncmp(func->Name, "__", 2)) {
draw_header = true;
@@ -4777,10 +4781,10 @@
}
for(i = 0; i < 2; i++)
{
- func = &lib->Functions[0];
- func_end = &lib->Functions[lib->Functions.size()];
+ func_begin = lib->Functions.begin();
+ func_end = lib->Functions.end();
- for(; func < func_end; func++)
+ for(lua_func_i func = func_begin; func != func_end; func++)
{
if(strncmp(func->Name, "__", 2)) {
continue;
@@ -4833,10 +4837,10 @@
}
for(i = 0; i < 2; i++)
{
- var = &lib->Variables[0];
- var_end = &lib->Variables[lib->Variables.size()];
+ var_begin = lib->Variables.begin();
+ var_end = lib->Variables.end();
- for(; var < var_end; var++)
+ for(lua_var_i var = var_begin; var != var_end; var++)
{
if(var->Type != NULL)
{
@@ -4879,9 +4883,9 @@
{
for(i = 0; i < 2; i++)
{
- func = &lib->Functions[0];
- func_end = &lib->Functions[lib->Functions.size()];
- for(; func < func_end; func++)
+ func_begin = lib->Functions.begin();
+ func_end = lib->Functions.end();
+ for(lua_func_i func = func_begin; func != func_end; func++)
{
if(strncmp(func->Name, "__", 2)) {
draw_header = true;
@@ -4900,10 +4904,10 @@
}
for(i = 0; i < 2; i++)
{
- func = &lib->Functions[0];
- func_end = &lib->Functions[lib->Functions.size()];
+ func_begin = lib->Functions.begin();
+ func_end = lib->Functions.end();
- for(; func < func_end; func++)
+ for(lua_func_i func = func_begin; func != func_end; func++)
{
if(!strncmp(func->Name, "__", 2)) {
continue;
@@ -4945,18 +4949,20 @@
//***Output Libraries
fputs("<dl>", fp);
fputs("<dt><b>Libraries</b></dt>", fp);
- lua_lib_h *lib = &lua_Libraries[0];
- lua_lib_h *lib_end = &lua_Libraries[lua_Libraries.size()];
- for(; lib < lib_end; lib++)
+ lua_lib_i lib_begin = lua_Libraries.begin();
+ lua_lib_i lib_end = lua_Libraries.end();
+ lua_lib_i lib;
+
+ for(lib = lib_begin; lib != lib_end; lib++)
{
fprintf(fp, "<dd><a href=\"#%s\">%s (%s)</a> - %s</dd>", lib->Name, lib->Name, lib->ShortName, lib->Description);
}
//***Output objects
- lib = &lua_Objects[0];
- lib_end = &lua_Objects[lua_Objects.size()];
+ lib_begin = lua_Objects.begin();
+ lib_end = lua_Objects.end();
fputs("<dt><b>Objects</b></dt>", fp);
- for(; lib < lib_end; lib++)
+ for(lib = lib_begin; lib != lib_end; lib++)
{
fprintf(fp, "<dd><a href=\"#%s\">%s</a> - %s</dd>", lib->Name, lib->Name, lib->Description);
}
@@ -4964,24 +4970,24 @@
//***Output libs
fputs("<dl>", fp);
- lib = &lua_Libraries[0];
- lib_end = &lua_Libraries[lua_Libraries.size()];
- for(; lib < lib_end; lib++)
+ lib_begin = lua_Libraries.begin();
+ lib_end = lua_Libraries.end();
+ for(lib = lib_begin; lib != lib_end; lib++)
{
fprintf(fp, "<dt id=\"%s\"><h2>%s (%s)</h2></dt>", lib->Name, lib->Name, lib->ShortName);
//Last param is something of a hack to handle lib derivs
- output_lib_meta(fp, lib, lib->Derivator > -1 ? &lua_Libraries[lib->Derivator] : NULL);
+ output_lib_meta(fp, &(*lib), lib->Derivator > -1 ? &lua_Libraries.at(lib->Derivator) : NULL);
}
//***Output objects
- lib = &lua_Objects[0];
- lib_end = &lua_Objects[lua_Objects.size()];
- for(; lib < lib_end; lib++)
+ lib_begin = lua_Objects.begin();
+ lib_end = lua_Objects.end();
+ for(lib = lib_begin; lib != lib_end; lib++)
{
fprintf(fp, "<dt id=\"%s\"><h2>%s - %s</h2></dt>", lib->Name, lib->Name, lib->Description);
//Last param is something of a hack to handle lib derivs
- output_lib_meta(fp, lib, lib->Derivator > -1 ? &lua_Objects[lib->Derivator] : NULL);
+ output_lib_meta(fp, &(*lib), lib->Derivator > -1 ? &lua_Objects.at(lib->Derivator) : NULL);
}
fputs("</dl>", fp);
}
Index: code/parse/lua.h
===================================================================
RCS file: /home/fs2source/cvsroot/fs2_open/code/parse/lua.h,v
retrieving revision 2.16
diff -u -r2.16 lua.h
--- code/parse/lua.h 15 Feb 2006 07:19:50 -0000 2.16
+++ code/parse/lua.h 22 Mar 2006 19:13:56 -0000
@@ -82,6 +82,9 @@
extern std::vector<lua_lib_h> lua_Libraries;
extern std::vector<lua_lib_h> lua_Objects;
+typedef std::vector<lua_func_hh>::iterator lua_func_i;
+typedef std::vector<lua_var_hh>::iterator lua_var_i;
+typedef std::vector<lua_lib_h>::iterator lua_lib_i;
//Library class
//This is what you define a variable of to make new libraryes
Index: code/parse/scripting.h
===================================================================
RCS file: /home/fs2source/cvsroot/fs2_open/code/parse/scripting.h,v
retrieving revision 2.12
diff -u -r2.12 scripting.h
--- code/parse/scripting.h 31 Jan 2006 06:46:14 -0000 2.12
+++ code/parse/scripting.h 20 Mar 2006 22:44:22 -0000
@@ -59,6 +59,7 @@
//Init functions for langs
int CreateLuaState();
+ int CreateLuaState(const script_lua_lib_list *libraries);
//int CreatePyState(const script_py_lib_list *libraries);
//Get data
|
|
|
I don't even want to touch this... The code (AFAIK) works exactly as it should, and the pointers past the end of the array are intentional - it's simpler than subtracting one to make them go to the end of the array. I'm not one to trust C++ stuff like iterators, whose use seems rather vague and needlessly complicate a process, so I didn't use 'em. But if that's the only way to get things cross-platform, I guess it's fine if they're used, since any speed difference will not be as big of a deal. Just make sure that you're working with up-to-date code. The patch you provided seems to be rather out-of-date. |
|
|
It's up to date with CVS, and C++ iterators aren't vague. vector.begin() is basically the same as vector[0], and vector.end() is the same as one past the end. It's right there in the documents. Thus the code should be functionally the same. Hence I used the iterators, which should be the same as the already existing code (aside from far better safety), instead of front() and back(). The Lua version part is to make it work correctly (at least on VS2005) with support for Lua 5.1. Either luaopen_io or luaopen_string was crashing. The only line that's functionally different in the patch should be lua_lib_h *libobj = NULL; changed to lua_lib_i libobj_begin = lua_Libraries.begin(); but you can't assign a random NULL to an iterator, not that that seems to actually be used anyway. The problem isn't that necessarily that there could be a pointer past the end of the array, it's using the array itself to do it. Iterators are simpler and cleaner than trying to hack around with conditional pointer math to make it valid. |
|
|
Well, if you can modify the current source or provide a file I can diff (I've had no experience with patches on Windows), I'll commit the fix. |
|
|
Why would I provide a file you can diff? That's already a diff file. At worst, you could download TortoiseCVS or something similar for Windows to apply it with a convenient GUI and no fuss. If you want me to provide a pre-patched file after two weeks, after I lose my Windows installation again, and that I'd have to redownload a copy of the CVS repository just for that, then you're just going to have to specifically ask for that, as that's going to take quite a lot of time to download on dialup which is needed for other things. If you don't know about patching 'on Windows', it's the same as patching on any other OS, including that the same utilities have been ported and are available. |
|
|
I looked into this and I think I applied this code correctly to the latest CVS. Would you like it? How should I get it to you if not a patch? Should I just commit it? It compiles fine, but I don't know how to test it besides play missions with scripts. |
|
|
This should no longer be an issue with the new OO Lua code. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2006-03-22 20:04 | Zephiris | New Issue | |
| 2006-03-22 20:04 | Zephiris | File Added: lua.patch | |
| 2006-03-23 03:41 | WMCoolmon | Note Added: 0005219 | |
| 2006-03-24 00:51 | Zephiris | Note Added: 0005223 | |
| 2006-04-03 08:18 | WMCoolmon | Note Added: 0005273 | |
| 2006-04-05 15:59 | Zephiris | Note Added: 0005281 | |
| 2006-09-23 10:34 | Backslash | Note Added: 0006681 | |
| 2007-01-21 09:15 | WMCoolmon | Status | assigned => closed |
| 2007-01-21 09:15 | WMCoolmon | Note Added: 0007507 | |
| 2007-01-21 09:15 | WMCoolmon | Resolution | open => fixed |
