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.

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!

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.

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))".

Yep, I am validating the results.