While loop is causing a stack dump and the program to crash in C++

Question

I am just getting back into programming in C++. I wrote a program that is supposed to read hex colors from a file, mathematically compare them to an array of colors identified within the program to determine which one is the closest, and then write the original color and closest color to a file. For some reason after writing about 62,000 lines or so the program craps out with a stack dump. The file I'm reading from has about 16 million colors in it. I was hoping someone might be able to point me in the right direction with my code to resolve this issue.

The code is below, I did not paste the arrays for red, green, blue, or pantonehexcode; but you can assume they are array with numerical and hexadecimal string values respectively.

    string line;
string hexcolor, r_hex, g_hex, b_hex;

const char delim[] = " ;";
float *cie1 = new float[3];
float *cie2 = new float[3];

float r ;
float g ;
float b ;
float currentClosestVal = 1000000;
float challengeClosestVal;
int currentClosestIndex;

    ifstream file ("hexcolormaplist.txt");
if (file.fail()) 
        {cout << "Error opening infile file"; return 0;}
ofstream ofile("tpxmap.txt");
if (ofile.fail()) 
        {cout << "Error opening ofile file"; return 0;}

bool newline = true;
//Comparing colors variables
int i, k;
double Kl, K1, K2, Sl, SC, SH, dL, dA, dB, dC, dH, c1, c2;

getline (file,line);
char * cline = new char [line.length()+1];



while(newline == true){

    currentClosestVal = 1000000;
    std::strcpy (cline ,line.c_str());
    hexcolor = strtok(cline, delim);
    r_hex = strtok(NULL, delim);
    g_hex = strtok(NULL, delim);
    b_hex = strtok(NULL, delim);


    r = (float)atof(r_hex.c_str());
    g = (float)atof(g_hex.c_str());
    b = (float)atof(b_hex.c_str());

    cie1 = rgb2lab (r, g, b);



    for (i = 0; i < 2100; i++)
    {
        cie2 = rgb2lab (red[i], green[i], blue[i]);
        //challengeClosestVal = pow(cie1[0] - cie2[0], 2.0) + pow(cie1[1] - cie2[1], 2.0) + pow(cie1[2] - cie2[2], 2.0);
        dL = cie1[0] - cie2[0];
        dA = cie1[1] - cie2[1];
        dB = cie1[2] - cie2[2];
        c1 = sqrt(cie1[1] + cie1[2]);
        c2 = sqrt(cie2[1] + cie2[2]);
        dC = c1 - c2;
        dH = sqrt(pow(dA, 2) + pow(dB,2) - pow(dC,2));
        Kl = 2;
        K1 = .048;
        K2 = .014;
        Sl = 1;
        SC = 1 + K1*c1;
        SH = 1 + K2*c1;

        challengeClosestVal = sqrt(pow(dL/(Kl*Sl), 2) + pow(dC/(Kl*SC),2) + pow(dH/(Kl*SH), 2));


        if(challengeClosestVal < currentClosestVal){
            currentClosestIndex = i;
            currentClosestVal = challengeClosestVal;
        }
    }

    ofile << hexcolor <<"; " << pantoneHexCodes[currentClosestIndex] <<";"<<endl; // prints The pantone color comparator
    line = "";
    newline = getline (file,line);

}//end of while loop

//close files
file.close();
ofile.close();
return 0;
}

float *rgb2lab (float r, float g, float b){

    float var_r, var_g, var_b;
    double X, Y, Z, var_X, var_Y, var_Z;
    float ref_X =  95.047;  //Observer= 2°, Illuminant= D65
    float ref_Y = 100.000;
    float ref_Z = 108.883;
    double cieL, cieA, cieB;
    float *cie = new float[3];



    //Convert RGB to XYZ
    //First set RGB values between 0-1
    var_r = r/255;
    var_g = g/255;
    var_b = b/255;

    if ( var_r > 0.04045 )
            var_r = pow( ( var_r + 0.055 ) / 1.055 , 2.4);
    else
            var_r = var_r / 12.92;

    if ( var_g > 0.04045 )
            var_g = pow( ( var_g + 0.055 ) / 1.055 , 2.4);
    else
            var_g = var_g / 12.92;
    if ( var_b > 0.04045 )
            var_b = pow( ( var_b + 0.055 ) / 1.055 , 2.4);
    else
            var_b = var_b / 12.92;

    var_r = var_r * 100;
    var_g = var_g * 100;
    var_b = var_b * 100;

    //Convert RGB to XYZ
    //Observer. = 2°, illuminant = D65
    X = var_r * 0.4124 + var_g * 0.3576 + var_b * 0.1805;
    Y = var_r * 0.2126 + var_g * 0.7152 + var_b * 0.0722;
    Z = var_r * 0.0193 + var_g * 0.1192 + var_b * 0.9505;

    //cout << "X: "<<X  <<"  Y: "  <<Y <<"  Z: "<<Z << endl;


    // Convert XYZ to CIELab
    var_X = X / ref_X;          //ref_X =  95.047   Observer= 2°, Illuminant= D65
    var_Y = Y / ref_Y;          //ref_Y = 100.000
    var_Z = Z / ref_Z;          //ref_Z = 108.883
    //cout << "var_X: "<<var_X  <<"  var_Y: "  <<var_Y <<"  var_Z: "<<var_Z << endl;

    if ( var_X > 0.008856 ) {
            var_X = pow(var_X, .3333333); }
    else
            var_X = ( 7.787 * var_X) + ( 16 / 116 );
    if ( var_Y > 0.008856 ){
            var_Y = pow(var_Y, .3333333); }
    else
            var_Y = ( 7.787 * var_Y) + ( 16 / 116 );
    if  ( var_Z > 0.008856 ){
            var_Z = pow(var_Z,  .3333333); }
    else
            var_Z = ( 7.787 * var_Z) + ( 16 / 116 );


    cieL = ( 116 * var_Y ) - 16;
    cieA = 500 * ( var_X - var_Y );
    cieB = 200 * ( var_Y - var_Z );

    //cout << "L: "<<cie[0]  <<"  a: "  <<cie[1] <<"  b: "<<cie[2] << endl;
    cie[0] = cieL;
    cie[1] = cieA;
    cie[2] = cieB;
    //cout << "L: "<<cie[0]  <<"  a: "  <<cie[1] <<"  b: "<<cie[2] << endl;
    return cie;

}

Did you compile with debug symbols? If so, was there a core file? Have you examined said core file with a debugger? Is the crash repeatable? Does it happen at the same iteration each time? If you're on linux, the `-g` flag to the compiler will add debug symbols, and to examine the core - `gdb ` — Macattack, Jul 25 '13 at 19:08
Sorry about that. I cleaned it up a bit, I've never pasted code here before. — user1370238, Jul 25 '13 at 19:08
can you post the backtrack and the input on the failing line? — cdmh, Jul 25 '13 at 19:15
I'm not familiar with the term backtrack. Googled it but don't know what you mean. The last line it writes is the comparison of line 62,386. I've change the order of the file several times and it always stops there. The last line read looks like this (as does every other line): _07A929; 7; 169; 41;_ — user1370238, Jul 25 '13 at 19:25

Doug · Accepted Answer · 2013-07-25T19:45:30.243

In your rgb2lab function you create a new float[3]. This happens with every call to this function. Looking at your code, I do not see where the memory for this is freed up with a call to delete anywhere.

It is good programming practice that every new is paired with a delete. It is also more important that the delete is being called in every execution path of the program if something is created with new.

What is happening in your code is that your rgb2lab function is being called 2100 times for every line written to file. You are saying that your program craps out after about 62000 lines being written to file. In this case, the rgb2lab function is being called 130,200,000 times and is leaking memory every single time.

EDIT:

You have a couple of spots where you are calling new that you should delete, but the biggest offender is the 2100 iteration for loop that calls the rgb2lab function over and over again. Since you are using the dynamically allocated array that the function returns, just free the memory when you are done using it.

for (i = 0; i < 2100; i++)
{
    cie2 = rgb2lab (red[i], green[i], blue[i]);
    //challengeClosestVal = pow(cie1[0] - cie2[0], 2.0) + pow(cie1[1] - cie2[1], 2.0) + pow(cie1[2] - cie2[2], 2.0);
    dL = cie1[0] - cie2[0];
    dA = cie1[1] - cie2[1];
    dB = cie1[2] - cie2[2];
    c1 = sqrt(cie1[1] + cie1[2]);
    c2 = sqrt(cie2[1] + cie2[2]);
    dC = c1 - c2;
    dH = sqrt(pow(dA, 2) + pow(dB,2) - pow(dC,2));
    Kl = 2;
    K1 = .048;
    K2 = .014;
    Sl = 1;
    SC = 1 + K1*c1;
    SH = 1 + K2*c1;

    challengeClosestVal = sqrt(pow(dL/(Kl*Sl), 2) + pow(dC/(Kl*SC),2) + pow(dH/(Kl*SH), 2));


    if(challengeClosestVal < currentClosestVal){
        currentClosestIndex = i;
        currentClosestVal = challengeClosestVal;
    }
}
delete [] cie2;  // <-- Free up the memory here!

Thank you I appreciate that. Where would I put the delete if I am trying to return the array I have created before I delete it? — user1370238, Jul 25 '13 at 19:32
You should place the `delete []` after you are finished using the returned array. Allocating and deleting the array will slow down your program. — Thomas Matthews, Jul 25 '13 at 19:45

score 1 · Answer 2 · answered Jul 25 '13 at 19:43

I don't see why you are using the new operator (except you come from a Java or C# background). You can pass results without using the new operator.

For example:

void rbg_to_lab (float r, float g, float b,
                 float& cie[3])
{
 // ...
}

By passing the cie array as a reference you can modify the caller's variable directly with allocating any new memory objects (and not having to delete the memory either.).

Another solution is to create a struct for the cie values and return the struct:

struct Cie_Values
{
  float cie_1;
  float cie_2;
  float cie_3;
};

Cie_Values rgb_to_lab(float r, float g, float b)
{
   Cie_values cie;
   // Perform conversion
   return cie;
}

While loop is causing a stack dump and the program to crash in C++

2 Answers2