Jump to content
  • Advertisement
DividedByZero

C++ Help initialising struct using memory offset please

Recommended Posts

Hi Guys,

I have a struct of int's which I am trying to initialise by pushing and popping a vector of integers.

All is working well if I initialise manually from the vector (for example)

options.vsync = boolQuery(optionsList.back(), mem);
optionsList.pop_back();

 

Instead of doing this for every member I am trying to use a simple 'for loop'. (Ignore the name boolQuery - this actually returns an int 0 for false, 1 for true, and -1 for indeterminate).

for (int i = 0; i < optionsList.size(); i++)
{
	int value = boolQuery(optionsList.back(), mem);
	optionsList.pop_back();
	memcpy(&options + (i * sizeof(int)), &value, sizeof(int));
}

 

This works for the first member but the remainder stay uninitialised.

Debugging shows that the expected values are occurring during the loop, but they just aren't getting stored in the member integers.

It looks as though the way I am trying to access the memory for the memcpy seems to be wrong.

Any advice would be hugely appreciated.

Share this post


Link to post
Share on other sites
Advertisement

That's because the expression

&options

return s pointer of type Options (or whatever the type of that struct of ints is), which when added to will produce an address that is offset by sizeof(options), which follows pointer arithmetic rules.

A more correct version of your second loop could be:

int* destinationInt = (int*)&options;
for (int i = 0; i < optionsList.size(); i++)
{
    int value = boolQuery(optionsList.back(), mem);
    optionsList.pop_back();
    *(destinationInt + i) = value;
}

But this very much assumes your struct hold only ints and is a POD type (no vtable, hidden paddings etc). I hope you know what you're doing.

Share this post


Link to post
Share on other sites

Do I understand correctly that options is just a struct?

In that case the problem is probably here:

&options + (i * sizeof(int))

With this you are not adding 1 * sizeof(int) to the address given by &options, but you are adding i * + sizeof(int) * sizeof(whatever type options is). In other words, after the first iteration, you are copying your integers all over the place, except inside your struct.

It might work if you cast &options to a char*. In any case, I would strongly suggest to reconsider what you are doing. There are for sure better ways to initialize your structure (more readable and less bug-prone).

Cheers!

Share this post


Link to post
Share on other sites

Thanks for the suggestions guys (+1'd you both).

This did the job;

int optionListSize = optionsList.size();
int * destInt = (int*)&options;
for (int i = 0; i < optionListSize; i++)
{
	int value = boolQuery(optionsList.back(), mem);
	optionsList.pop_back();
	*(destInt + i) = value;
}

In this case the struct will always be integers only. So I don't foresee any huge issues.

But I certainly wouldn't do this with mixed data types, especially where pointers etc are concerned.

It seemed like a more efficient option to populate the struct this way as I am reading parameters from a file.

So I am now 'pushing' the required parameters on to a vector and then reading them back (in reverse order) to fill the struct. This way I can just push new parameters (and add to the struct definition of course) and read back with minimal fuss, thanks to your help.

Share this post


Link to post
Share on other sites

I think just doing it member by member is a much better idea, it should only be a single line of code per parameter and can support multiple types and handles errors. Do you really want to always use "boolQuery(int, mem)"?

Casting a struct to a pointer like that will prevent the compiler from warning you if its actual contents does not match what you expect, just to save a few lines. It won't be meaningfully "more efficient" for the computer, even for large amounts of data.

For example, what happens if "optionsList" is the wrong size for some reason? Are you validating it? Because from the code you have it looks to me like it will start overwriting (likely with no error/warning!) whatever happens to be in memory after options, might be a mistake (changed the options, forgot to delete the file from disk), but also the sort of thing hackers like to find.

At the very least something like a "if (sizeof(options) != optionsList.size() * sizeof(int))".

 

 

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

GameDev.net is your game development community. Create an account for your GameDev Portfolio and participate in the largest developer community in the games industry.

Sign me up!