Custom API function causing segfaults ... Please help?

[AlexYst]

For a personal project of mine, it'd be very helpful to have access to the C++ read_seed() function from the Lua API. Today, I finally sat down to try to make that happen. I think I got rather close, but whenever this API function is called, it causes a segmentation fault. I'm sure it's some really simple problem, but as I'm not familiar with C++, I'm not sure what I'm doing wrong. I've tried several attempts at resolving the issue by converting the troublesome variable to different types because of other examples in the code, mostly causing other things to break in the process, but honestly, I don't think they type is necessarily the problem. My understanding is that a segfault is caused when memory that shouldn't be accessed is accessed anyway, but I don't see anywhere in my short code segments that I think should cause that. I'm not deallocating variables before using them, or even at all. Anyway, I could really use some help if someone is feeling generous with their time.

I've added this line into the ModApiServer class in the "scr/script/lua_api/lserver.h" file:

Code: Select all

static int l_read_seed(lua_State *L);

I've added this line into the ModApiServer::Initialize() function definition in the "scr/script/lua_api/lserver.cpp" file:

Code: Select all

API_FCT(read_seed);

Finally, my main code is just a new function added to the "scr/script/lua_api/lserver.cpp" file called ModApiServer::l_read_seed(). Originally, it looked like this:

Code: Select all

int ModApiServer::l_read_seed(lua_State *L)
{
	NO_MAP_LOCK_REQUIRED;
	const char * seed = luaL_checkstring(L, 1);
	lua_pushstring(L, (char*)read_seed(seed));
	return 1;
}

Because of the segfault though, I broke it up into several lines to try to isolate exactly what I was doing wrong. I now looks like this:

Code: Select all

int ModApiServer::l_read_seed(lua_State *L)
{
	NO_MAP_LOCK_REQUIRED;
	const char * seed = luaL_checkstring(L, 1);
	const u64 numeric_seed = read_seed(seed);
	const char * digits = (char*)numeric_seed;
	lua_pushstring(L, digits);
	return 1;
}

Enabling only one line at a time, I find that the function doesn't cause any segfaults without that last line. Something in the lua_pushstring() function causes the segfault, so I can only assume that I'm passing in a bad value or something. I'm not sure what I can do to fix the value aside from altering its type though, which as mentioned previously, has caused other problems.

[Pyrollo]

Code: Select all

	const char * digits = (char*)numeric_seed;

That looks quite weird. numeric_seed is a u64 and you cast it to a char pointer (so pointing to kind of random memory area). If you want to have the string representation of your u64 (ie "123456789" for example), you should use sprintf function to do that.

I don't really catch the idea. Your function would be useful only to convert alphabetic seeds to number seeds represented as a string. Is this what you need ?

You could also push the result as integer on Lua stack, but unfortunately the Lua number representation can't handle such precision (64 bits, integer), it looses less significant digits.

[AlexYst]

If you want to have the string representation of your u64 (ie "123456789" for example), you should use sprintf function to do that.

I just looked up the sprintf(), and it seems to write to a buffer of some sort. That sounds like outputting to the console or something, unless I'm misunderstanding. Would there be a way to use a variable as the buffer or something?

I don't really catch the idea. Your function would be useful only to convert alphabetic seeds to number seeds represented as a string. Is this what you need ?

Yes. That's exactly what I need.

You could also push the result as integer on Lua stack, but unfortunately the Lua number representation can't handle such precision (64 bits, integer), it looses less significant digits.

That's exactly why I cast the u64 into a string first. Lua's string length is more than capable of handling the seed in its full length if it's represented in string form.

numeric_seed is a u64 and you cast it to a char pointer (so pointing to kind of random memory area).

Hmm. I don't think I fully understood what you meant by that at first. I think I get it now though. When I cast the u64 to a char*, it's not generating a new char* that holds the string representation of the integer. Instead, it's using the value of the u64 as the address of a pointer. Is that correct? It certainly would explain the segfault. Until the pointer gets used, no error occurs, but when it does, which happens within the lua_pushstring() function when I pass that pointer, this memory address which was never allocated (... I can't remember the right term ... it's the opposite of "freed" though ...) gets accessed, at which point a segfault would occur.

Thank you very much for the insight, Pyrollo! I'll see what I can figure out about converting integers into strings, starting with researching the sprintf() function and buffers.

EDIT:

Alright, I think I have it figured out. Thanks again, Pyrollo! My final function ended up as follows, in case anyone is curious now or in the future. It's always a bit annoying when I see a topic like this about a problem that was solved and the final solution is still nowhere to be seen.

Code: Select all

int ModApiServer::l_read_seed(lua_State *L)
{
	NO_MAP_LOCK_REQUIRED;
	const char * seed = luaL_checkstring(L, 1);
	char* buffer = new char[20];
	sprintf(buffer, "%lu", read_seed(seed));
	lua_pushstring(L, buffer);
	return 1;
}

I'm not sure if there's a way to dynamically figure out how long the character buffer should be without putting in a bunch of mathematical logic with twenty separate cases, one for each appropriate length. Or I guess a loop could be used to repeatedly divide the number by ten until the value is less than ten, as a way to count the digits. In any case, I have a notes file about what I've learned about Minetest and need to remember, and it tells me the maximum seed is 18446744073709551615, which is 20 digits long, so a buffer of 20 should always be long enough, and sometimes too long. My calculator isn't precise enough to check this value (which should be 2^64-1), but the value should be correct. Experimentation has shown that having the buffer be too long doesn't seem to cause noticeable problems for now. It's probably less memory-efficient, but that's fine for my project. This isn't code to be put into the official Minetest engine, so making it as clean and efficient as possible isn't a priority.

[rubenwardy]

A buffer is just a string that you can write into, like so:

Code: Select all

char buffer[30];
snprintf(&buffer[0], 30, "%d", my_number);

30 is the maximum size of the string that could be produced

This is a bad way to do it, however. It would be better to use std::to_string like so:

Code: Select all

std::string seed = std::to_string(numeric_seed);

[AlexYst]

It looks like I was a bit too late to edit my post. That does look a lot cleaner than what I just wrote though, and much closer semantically in my intent, so I'll exchange that in my code. Thank you Rubenwardy!

[rubenwardy]

Your code there also has a memory leak - avoid using new

But yeah, std::to_string is much nicer to use

[Pyrollo]

But yeah, std::to_string is much nicer to use

You are right, sprintf is the old C (non C++) way.

[Pyrollo]

When I cast the u64 to a char*, it's not generating a new char* that holds the string representation of the integer. Instead, it's using the value of the u64 as the address of a pointer. Is that correct? It certainly would explain the segfault.

Exactly, you fool the processor telling it a char string should be found at this address which is false. In addition, it's very likely that the process does not even have the right to read that address. This is what a segfault is.

Even if C++ now have kind of high level stuff, it is still a low level language. It is important to keep in mind that there is almost no safeguard.

[AlexYst]

Wow. I had no idea how easy it is to accidentally introduce segfaults and memory leaks. I have a lot to learn before I can do anything useful for the Minetest project.

I'm still not ready to take on any large projects, especially while I'm still in school and have coursework to do, but this has been a nice little learning experience to let me test the waters a bit. I don't have the attention span or motivation for "hello world" scripts, so I sort of have to jump into small practical projects such as this to learn anything.

Anyway, thank you both for helping me learn a bit about C++!