Sign in to follow this  
StonieJ

static_cast

Recommended Posts

Does static_cast cause the argument to be converted right there on the spot, or does it only store the cast into the LHS? For example, this code is throwing an access violation error when it gets to the first delete[] statement:
// real_out and im_out are double** arrays

for (int i = 0; i < height; i++)
    for (int j = 0; j < width; j++)
    {
        fftData[i][k] = static_cast<float>(real_out[i][j]);
        fftData[i][k + 1] = static_cast<float>(im_out[i][j]);
    }

    for (int i = 0; i < MRCHeight; i++)
    {
        delete[] real_out[i];  <=== ERROR HERE!  ACCESS VIOLATION!
        delete[] im_out[i];
    }

    delete[] real_out;
    delete[] im_out;


However, if I comment out the static_cast statements, no problems. Is this always the case? For some reason I thought the cast was only applied to a return value...not the actual value itself. If this really is what is causing the problem, what cast should I use to make sure the fftData value is properly assigned as a float?

Share this post


Link to post
Share on other sites
cant use my hands ;-) .. post deleted


try this instead:

for (int j = 0; j < width; j++)
{
fftData[i][k] = (float)(real_out[i][j]);
fftData[i][k + 1] = (float)(im_out[i][j]);
}

[Edited by - hoLogramm on November 8, 2004 4:48:57 AM]

Share this post


Link to post
Share on other sites
Quote:
Original post by StonieJ
For some reason I thought the cast was only applied to a return value...not the actual value itself.


That is correct. A cast actually creates a temporary rvalue of the target type. The original lvalue is not modified.
I suspect you are writing past the end of your fftData array, trashing the bookkeeping headers for the real_out and im_out arrays.

i.e. it's the assignment, not the cast which is at fault.

Share this post


Link to post
Share on other sites
Quote:
Original post by hoLogramm
try this instead:

for (int j = 0; j < width; j++)
{
fftData[i][k] = (float)(real_out[i][j]);
fftData[i][k + 1] = (float)(im_out[i][j]);
}


If the jist of your post is that old C casts are preferrable over the new C++ cast operators then I disagree very strongly. The C++ cast operators are much more explicit and you actually have to stop and think a second what kind of cast you really need/want rather than just casting away for the heck of it.

Share this post


Link to post
Share on other sites
There's almost certainly a bug in that original code because the first nested loops are equivalent to:
for (int i = 0; i < height; i++)
{
fftData[i][k] = static_cast<float>(real_out[i][width - 1]);
fftData[i][k + 1] = static_cast<float>(im_out[i][width - 1]);
}

The way it is at the moment you just keep overwriting the values width times.

Enigma

Share this post


Link to post
Share on other sites
no, it doesn't look like you're overwriting values... except for possibly i. Your indentation makes it look like the deletion loop happens inside the first for(i = 0; i < width...). However, I suspect that's just a cut-and-paste artifact.

Is MRCHeight == height ?
what type is real_out to begin with that you would want to cast?

Share this post


Link to post
Share on other sites
The code posted most certainly does overwrite values for width > 1. If you unroll the inner loop of the first double-loop:
for (int i = 0; i < height; i++)
{
/* unroll this loop
for (int j = 0; j < width; j++)
{
fftData[i][k] = static_cast<float>(real_out[i][j]);
fftData[i][k + 1] = static_cast<float>(im_out[i][j]);
}*/

fftData[i][k] = static_cast<float>(real_out[i][0]);
fftData[i][k + 1] = static_cast<float>(im_out[i][0]);
fftData[i][k] = static_cast<float>(real_out[i][1]);
fftData[i][k + 1] = static_cast<float>(im_out[i][1]);
fftData[i][k] = static_cast<float>(real_out[i][2]);
fftData[i][k + 1] = static_cast<float>(im_out[i][2]);
// ...
}



k never changes, so the values of fftData[i][k] and fftData[i][k + 1] are written multiple times.

Enigma

Share this post


Link to post
Share on other sites
I agree that using C++ style casts' is almost always preferable to C cast.

Just for the simple fact that you can much easier grep for them. ie finding all static_cast<float>(..) is alot easier to find that (float).

The latter would produce alot of false positives. for things like funciton prototypes and definitions with no parameters.

Cheers
Chris

Share this post


Link to post
Share on other sites
Crap, perhaps I should have posted the whole algorithm. I left out the k terms because I just got lazy, but I will include them now. Ultimately, this is what I'm trying to do:

1) Read in two arrays of data.
2) Merge them into a single array of data.

Shouldn't be too hard I'd think. Here is the complete, relevant code segment with my error code (i.e. I left out some stuff that simply wasn't vital):


void CEMData::FFT()
{
double **real_in = new double*[MRCHeight];
double **im_in = new double*[MRCHeight];
double **real_out = new double*[MRCHeight];
double **im_out = new double*[MRCHeight];

try
{
if (!real_in)
throw Exception("Possibly out of memory.");

if (!im_in)
throw Exception("Possibly out of memory.");


if (!real_out)
throw Exception("Possibly out of memory.");

if (!im_out)
throw Exception("Possibly out of memory.");

for (int i = 0; i < MRCHeight; i++)
{
real_in[i] = new double[MRCWidth];
im_in[i] = new double[MRCWidth];
real_out[i] = new double[MRCWidth];
im_out[i] = new double[MRCWidth];
}

for (int i = 0; i < MRCHeight; i++)
{
if (!real_in[i] || !im_in[i] || !real_out[i] || !im_out[i])
throw Exception("Possibly out of memory.");
}

// the imaginary input array should be all zeros
for (int i = 0; i < MRCHeight; i++)
ZeroMemory(im_in[i], sizeof(double) * MRCWidth);


// POPULATE INPUT ARRAYS (not shown)


// do the one-dimensional transform on the rows
for (int i = 0; i < MRCHeight; i++)
fft(MRCWidth, real_in[i], im_in[i], real_out[i], im_out[i]);

// permute the rows and store them in the input variables (since we will be inputting them again)
for (int i = 0; i < MRCHeight; i++)
{
for (int j = 0; j < MRCWidth; j++)
{
real_in[i][j] = real_out[j][i];
im_in[i][j] = im_out[j][i];
}
}

// do the one-dimensional transform on the rows again (which used to be the columns)
for (int i = 0; i < MRCHeight; i++)
fft(MRCWidth, real_in[i], im_in[i], real_out[i], im_out[i]);


// delete the input data since we're done with it
for (int i = 0; i < MRCHeight; i++)
{
delete[] real_in[i];
delete[] im_in[i];
}

delete[] real_in;
delete[] im_in;


// convert the transform data from double to float
// HERE IS WHERE THE PROBLEMS START OCCURRING

fftData = new float *[MRCHeight];

if (!fftData)
throw Exception("Possibly out of memory.");

for (int i = 0; i < MRCHeight; i++)
fftData[i] = new float[MRCWidth];

/* NOTE FOR THE INNER LOOP...YOU ONLY NEED THE FIRST W/2 + 1 COLUMS OF THE DATA...NOT THE ENTIRE WIDTH */

for (int i = 0; i < MRCHeight; i++)
{
int k = 0;

for (int j = 0; j < MRCWidth / 2 + 1; j++)
{
fftData[i][k] = static_cast<float>(real_out[i][j]);
fftData[i][k + 1] = static_cast<float>(im_out[i][j]);
k += 2;
}
}

// delete the original output data since we're done with it
for (int i = 0; i < MRCHeight; i++)
{
delete[] real_out[i]; <=== ERROR HERE!!!
delete[] im_out[i];
}

delete[] real_out;
delete[] im_out;
}

Share this post


Link to post
Share on other sites
Two things that look suspicious:
1.
    for (int i = 0; i < MRCHeight; i++)
{
for (int j = 0; j < MRCWidth; j++)
{
real_in[i][j] = real_out[j][i];
im_in[i][j] = im_out[j][i];
}
}

This will only work if MRCHeight == MRCWidth otherwise you are reading out of bounds of real_out and im_out.
2.
        int k = 0;

for (int j = 0; j < MRCWidth / 2 + 1; j++)
{
fftData[i][k] = static_cast<float>(real_out[i][j]);
fftData[i][k + 1] = static_cast<float>(im_out[i][j]);
k += 2;
}

fftData[i] is defined to be an array of size MRCWidth but you are writing ((MRCWidth / 2) + 1) * 2 items to it, or MRCWidth + 2 items, so you are overwriting the bounds of the array.

Enigma

Share this post


Link to post
Share on other sites
Quote:
Original post by Enigma
There's almost certainly a bug in that original code because the first nested loops are equivalent to:
for (int i = 0; i < height; i++)
{
fftData[i][k] = static_cast<float>(real_out[i][width - 1]);
fftData[i][k + 1] = static_cast<float>(im_out[i][width - 1]);
}


I don't see how this code overwrites any data.

And, while the new listing writes past the end of the array, it's very odd that it doesn't crash without the cast.

Share this post


Link to post
Share on other sites
The code I posted and you quoted does not overwrite values. The original code was:
for (int i = 0; i < height; i++)
for (int j = 0; j < width; j++)
{
fftData[i][k] = static_cast<float>(real_out[i][j]);
fftData[i][k + 1] = static_cast<float>(im_out[i][j]);
}

Which simply writes real_out[i][0] to fftData[i][k] and then immediately overwrites that by writing real_out[i][1] to fftData[i][k] on the next iteration of the loop. The code I posted and you quoted is functionally equivalent but rather that overwriting each time it simply writes the final value directly.

The point is rather moot now that StonieJ has posted the real code he was using. While attempting to cut down code for forum posting is a Good Thing™ you have to be careful to retain the real problem. I originally highlighted the loop because I assumed he was simply using the wrong variable to index into fftData.

Enigma

Share this post


Link to post
Share on other sites
Use asserts!!!

If your program causes an access violation when calling delete then you HAVE written to memory that you shouldn't have. In ~95% of cases it's past the end of an array.
The only question is where. If you comment out a line of code and it works, then that line of code is probably where it was overwriting an array.

Make sure you always use asserts to verify that you are within the bounds or your array. Add them now and it should show you where problem is.

Notice how no one ever posts code that causes an access violation on delete, where they did actually use asserts? That's because the asserts found the problem for those people already.

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

Sign in to follow this