This is the exact C++ code in question:threads = new glLibInternal_MetaWorker*[glLibInternal_numof_threads];
for (int i=0;i<glLibInternal_numof_threads;++i) threads = new glLibInternal_MetaWorker(this,mesh);
int i = 0;
for (int x=0;x<grid_res[0];++x) { for (int y=0;y<grid_res[1];++y) { for (int z=0;z<grid_res[2];++z) {
threads->owned_coords.push_back(x);
threads->owned_coords.push_back(y);
threads->owned_coords.push_back(z);
++i;
if (i==glLibInternal_numof_threads) i = 0;
}}}The variable glLibInternal_numof_threads is an external global int that, at runtime, is set to 12. grid_res[3] is an int array, with all elements equal to 16.
As general good practice, I ran this through static code analysis. Visual Studio gives (on the fifth line in the above source):
warning C6385: Invalid data: accessing 'threads', the readable size is 'int glLib::glLibInternal_numof_threads*4' bytes, but '16' bytes might be read
My question is how? Am I missing something obvious?
Thanks,
Ian
[size="1"]And a Unix user said rm -rf *.* and all was null and void...|There's no place like 127.0.0.1|The Application "Programmer" has unexpectedly quit. An error of type A.M. has occurred. [size="2"]
Probably because of the shadowing of the variable "i". Shadowing is generally a bad idea and can lead to subtle bugs. I suggest naming the outer loop counter something different, or alternatively not using "i" as the name of the secondary counter.
True. There's no shadowing here. The only potential problem i see here is when glLibInternal_numof_threads is zero. In that case the second loop (in which the warning is triggered) would still access threads[0], despite there being no threads. Also, static analyzers aren't perfect, so maybe the static analyzer is just confused by the way wraparound is handled for i. Maybe try >= instead of ==.
[quote name='RulerOfNothing' timestamp='1339315695' post='4947849']
What outer loop? The loop that uses 'i' as a counter is only one line long.
Heh, good catch. That code is bloody hard to read.
[/quote]
I can only second this - code analysis and a compiler will do so much, but to a human being reading it, it just looks wrong.
Direct3D has need of instancing, but we do not. We have plenty of glVertexAttrib calls.
Even some whitespace helps - try this for more readable:threads = new glLibInternal_MetaWorker *[glLibInternal_numof_threads];
for (int i = 0; i < glLibInternal_numof_threads; ++i)
threads = new glLibInternal_MetaWorker (this, mesh);
int i = 0;
for (int x = 0; x < grid_res[0]; ++x)
{
for (int y = 0; y < grid_res[1]; ++y)
{
for (int z = 0; z < grid_res[2]; ++z)
{
threads->owned_coords.push_back (x);
threads->owned_coords.push_back (y);
threads->owned_coords.push_back (z);
++i;
if (i == glLibInternal_numof_threads) i = 0;
}
}
}
Direct3D has need of instancing, but we do not. We have plenty of glVertexAttrib calls.
