This forum is a read-only archive of the Gammon Software forum (2000–2026). No new posts can be made. Search the archive.

Avoid STRFREE & str_alloc via Lua?

Posted by Darwin on Fri 25 Jan 2008 01:57 AM — 12 posts, 39,336 views.

Darwin USA #0
I have, what I believe to be, Slash's random equipment code in my source. I've ported the name and effects part out to Lua and call the functions via a modified version of the call_va function found in the PiL book.

After all the effects are added it runs the following lines to add the random names to the items:
    sprintf( buf, obj->short_descr, name );
    STRFREE( obj->short_descr );
    obj->short_descr = str_alloc( buf );

    sprintf( buf, "%s", obj->short_descr );
    STRFREE( obj->name );
    obj->name = str_alloc( buf );

    STRFREE( obj->description );
    sprintf( buf, "%s lies here.", capitalize( obj->short_descr ) );  
    obj->description = str_alloc( buf );

The name and buf variables are initialized as
   char *name = "[random]";
   char buf[MAX_STRING_LENGTH];

The name variable is passed byref to Lua which generates the name and passes it back to C, which then is formatted into a buffer variable and applied to the object.

What I'm wondering is, could I skip the STRFREE and str_alloc calls and just pass the name, short_descr and description to Lua and set them there, or would there be some kind of a mess from not freeing the strings first?

I'm wanting to avoid using the buf and name variables and any unnecessary calls to STRFREE and str_alloc.
Nick Gammon Australia Forum Administrator #1
I think you are doing it correctly.

I think the str_alloc function shares exactly the same descriptions between various items based on a hash table, so you can't just change it without freeing and reallocating.

If the name is generated by Lua (is that what you meant?) and is being returned a const char * field, I think you have to use it fairly soon, or the memory it is pointing to might be allocated to something else.
Darwin USA #2
I'll elaborate a bit.
void call_va( lua_State *L, const char *func, const char *sig, ...)
{
	va_list v1;
	int narg, nres;
	
	va_start(v1, sig);
	lua_getglobal(L, func);
	
	// push argument
	for(narg = 0; *sig; narg++)
	{
		luaL_checkstack(L, 1, "too many arguments");
		
		switch(*sig++)
		{
			case 'd':
				lua_pushnumber(L, va_arg(v1, double));
				break;
			
			case 'i':
				lua_pushinteger(L, va_arg(v1, int));
				break;
			
			case 's':
				lua_pushstring(L, va_arg(v1, char *));
				break;
			
			case '>':
				goto endargs;
			
			default:
				luaL_error(L, "invalid option (%c)", *(sig -1));
		}
	}
	endargs:
	
	nres = strlen(sig);
	
	if(lua_pcall(L, narg, nres, 0) != 0)
		luaL_error(L, "error calling '%s': %s", func, lua_tostring(L, -1));
	
	// results
	nres = -nres;
	while (*sig)
	{
		switch(*sig++)
		{
			case 'd':
				if(!lua_isnumber(L, nres))
					luaL_error(L, "wrong result type, double expected.");
				*va_arg(v1, double *) = lua_tonumber(L, nres);
				break;
			
			case 'i':
				if(!lua_isnumber(L, nres))
					luaL_error(L, "wrong result type, integer expected.");
				*va_arg(v1, int *) = lua_tointeger(L, nres);
				break;
			case 's':
				if(!lua_isstring(L, nres))
					luaL_error(L, "wrong result type, string expected.");
				*va_arg(v1, const char **) = lua_tostring(L, nres);
				break;
			
			default:
				luaL_error(L, "invalid option (%c)", *(sig -1));
		}
		nres++;
	}
	
	va_end(v1);
}

This is almost verbatim from the book. I had to add the lua_State to the arguments list.
This is used like this:
    else if (obj->item_type == ITEM_LIGHT)

    { 

        call_va(L_mud, "generate_random_item", "ii>s", obj->item_type, FALSE, &name);
// yadda yadda...
    sprintf( buf, obj->short_descr, name );
    STRFREE( obj->short_descr );
    obj->short_descr = str_alloc( buf );

    sprintf( buf, "%s", obj->short_descr );
    STRFREE( obj->name );
    obj->name = str_alloc( buf );

    STRFREE( obj->description );
    sprintf( buf, "%s lies here.", capitalize( obj->short_descr ) );
    obj->description = str_alloc( buf );

Which then calls the Lua function
function gr(list)
    return list[math.random(1, #list)]
end

function generate_random_item(iType, bool)

	if iType == itype.armor or iType == itype.artarmor then
		if bool == 1 then
			local x = math.random(1,#metalarmors)
            name = string.format("%s %s", gr(metalarmor_mods), metalarmors[x].name)
			ac = metalarmors[x].ac
			weight = metalarmors[x].weight
		else
			local x = math.random(1, #otherarmor_types)
            name = string.format("%s %s", gr(otherarmor_mods), otherarmor_types[x].name)
			ac = otherarmor_types[x].ac
			weight = otherarmor_types[x].weight
		end
		return name, ac, weight
	end
	
	if iType == itype.worn or iType == itype.artworn then
        return gr(badges)
	end
	
	if iType == itype.weapon or iType == itype.artweapon then
        local w = gr(weapons)
		return w.name, w.weight, w.roll, w.dam
	end
	
	if iType == itype.treasure or iType == itype.arttreasure then
		if bool == 1 then
            name = string.format("%s %s %s", gr(treasureadj1), gr(treasureadj2), gr(trinket))
		else
            name = string.format("%s %s", gr(treasureadj1), gr(treasureadj2))
		end
		return name
	end
	
	if iType == itype.light or iType == itype.artlight then
        name = string.format("%s %s", gr(treasureadj1), gr(treasureadj2))
		return name
	end
end

Name is returned to C containing something like (for a light item) "a warm onyx" which is then sprintf'd into the buf variable through the chosen item's generic short description such as %s orb, resulting in "a warm onyx orb".

I'm just wondering if I can just bypass the STRFREE and str_alloc calls and just pass &obj->short_descr to Lua instead of the name variable. Or will there be memory issues arrising from doing that?
Amended on Fri 25 Jan 2008 04:14 AM by Darwin
Nick Gammon Australia Forum Administrator #3
My first reaction is that I am not overly keen on seeing the dreaded "goto" in code. Instead of what you have done, why not change:


	for(narg = 0; *sig; narg++)


to


	for(narg = 0; *sig && *sig != '>'; narg++)


That breaks you out of the loop at the ">" sign, and avoids a goto.
Darwin USA #4
I wasn't too keen on the goto in the code. I was surprised that it was there, to be honest.
Nick Gammon Australia Forum Administrator #5
Quote:

I'm just wondering if I can just bypass the STRFREE and str_alloc calls and just pass &obj->short_descr to Lua instead of the name variable


You aren't passing 'name' to Lua, you are receiving it. That is different.

I suggest you read:

http://www.lua.org/manual/5.0/manual.html

In particular search for "tostring" and read this:


Because Lua has garbage collection, there is no guarantee that the pointer returned by lua_tostring will be valid after the corresponding value is removed from the stack.


In other words, the pointer you got from Lua will not necessarily stay valid for long. You need to do the str_alloc, as I said before.
Nick Gammon Australia Forum Administrator #6
I might add that if you don't, and it doesn't immediately crash, then you will have created one of those situations where the MUD crashes unexpectedly from time to time. It might be 15 minutes after the code that is really responsible is used, and be very very hard to track down.
Darwin USA #7
Ok, but is it still necessary to call STRFREE?
Nick Gammon Australia Forum Administrator #8
Yes, to decrement the count for the old string, otherwise it will stay in memory forever.
Darwin USA #9
Gotcha.

Alrighty, I've got to rethink what I'm doing. :)
David Haley USA #10
This is part of the joy of string handling in C -- every little reassignment has to be accompanied by the whole shebang of freeing and reallocating. This alone is a major reason for me to move to C++. :-)
Darwin USA #11
Another reason I was trying to go this route was to avoid all the 
char *var[MAX_STRING_LENGTH];
declarations.

I've managed to change those to just 
char *var;
and let Lua do all the memory allocating for the string. Although, I'm still using STRFREE and str_alloc to apply the new string to objects.

I figure that declaring 4 bytes is better than declaring 2 or 4k, most of which isn't necessary.