CodeGuru Home VC++ / MFC / C++ .NET / C# Visual Basic VB Forums Developer.com
Results 1 to 12 of 12
  1. #1
    Join Date
    Apr 2013
    Posts
    4

    crashes after displaying 100 bars??? help please.

    I have written a MFC dll to interact with an API and it is run by the target program. the first 100 bars are drawn properly but then the program ceases to operate. I am guessing I have a memory leak but I probably just dont understand what I am doing as I am quite new to programming MFC GDI. Any help is greatly appreciated. Here is my code that causes the trouble...

    The variables coming in are commented out to facilitate testing and so it would be displaying the same bars over and over because of this. it worrks for 100 bars, then crashes.


    void MainDlg::SS_UpdateChart()
    {
    // the chart plot is completely recalculated each time a new bar is shown
    // 300 is plottable height
    // 400 is plottable width, 39 bars
    int displayChartHeight = 300;

    // variables used to display the chart
    double displayPriceRangeHigh = 0;
    double displayPriceRangeLow = 10000;
    double pixelsPerDollar = 0;





    CDC * myDC; // the picture box context

    CPen * oldPen;

    CBrush * oldBrush;


    // set up the picture box
    myDC = GetDlgItem(IDC_STATICPIC1)->GetDC(); // get the item context

    oldPen = (CPen *) myDC->SelectStockObject(WHITE_PEN); // initialize
    myDC->SelectObject(oldPen);

    oldBrush = (CBrush *) myDC->SelectStockObject(WHITE_BRUSH); // initialize
    myDC->SelectObject(oldBrush);




    // the data for each bar is loaded into the chart array
    // for each movement of the bars back the range is recalculated
    // and the data is reformulated and converted to fill the chart bars

    // the current bar in is bar 40 to stage it
    chartBar[40].open = 15;//currentBarOpen;
    chartBar[40].high = 18;//currentBarHigh;
    chartBar[40].low = 12;//currentBarLow;
    chartBar[40].close = 13;//currentBarClose;
    chartBar[40].volume = 0;//currentBarVolume;
    chartBar[40].pressure = 0;//currentBarAveragePressure;



    // the displayed range of prices
    // the range of chart is determined by the highest and lowest of all the bars
    // being displayed at the time from second bar to the current bar being built
    // only include the range of the bars that exist when first starting the display

    // get the range on bars 2 to 40

    for( int a = 2; a <= 40; a = a + 1)
    {
    if ((chartBar[a].high != 0) && (chartBar[a].high != NULL))
    {
    if (displayPriceRangeHigh < chartBar[a].high)
    {
    displayPriceRangeHigh = chartBar[a].high;
    }
    }

    if ((chartBar[a].low != 0) && (chartBar[a].low != NULL))
    {
    if (displayPriceRangeLow > chartBar[a].low)
    {
    displayPriceRangeLow = chartBar[a].low;
    }
    }
    }




    // to avoid division by zero or null
    if ((displayPriceRangeHigh < 1) || (displayPriceRangeHigh == NULL))
    {
    displayPriceRangeHigh = 1;
    displayPriceRangeLow = 1;
    }
    if ((displayPriceRangeLow < 1) || (displayPriceRangeLow == NULL))
    {
    displayPriceRangeHigh = 1;
    displayPriceRangeLow = 1;
    }



    // get pixels per dollar
    pixelsPerDollar = displayChartHeight / (displayPriceRangeHigh - displayPriceRangeLow);



    // move bar data back one column and move current bar from 40 into bar 39
    // this makes bars 1 to 39 current for displaying

    for( int a = 2; a <= 40; a = a + 1)
    {
    chartBar[(a - 1)].open = chartBar[a].open;
    chartBar[(a - 1)].high = chartBar[a].high;
    chartBar[(a - 1)].low = chartBar[a].low;
    chartBar[(a - 1)].close = chartBar[a].close;
    chartBar[(a - 1)].volume = chartBar[a].volume;
    chartBar[(a - 1)].pressure = chartBar[a].pressure;
    }



    // clear the display for the new bars
    CBrush* blackBrush = new CBrush;

    blackBrush->CreateSolidBrush(blackColor);

    //oldBrush = myDC->SelectObject(blackBrush);
    myDC->SelectObject(blackBrush);

    myDC->FillRect(new CRect (0, 0, 460, 330), blackBrush);

    DeleteObject(blackBrush);

    myDC->SelectObject(oldBrush);


    // find colors to display for each bar space 1 column at a time
    // divide the 100% into the increments of the display

    int barPosition;
    int barOpen;
    int barHigh;
    int barLow;
    int barClose;


    for( int a = 1; a <= 39; a = a + 1)
    {

    barPosition = a;

    barOpen = ((chartBar[a].open - displayPriceRangeLow) * pixelsPerDollar);
    barHigh = ((chartBar[a].high - displayPriceRangeLow) * pixelsPerDollar);
    barLow = ((chartBar[a].low - displayPriceRangeLow) * pixelsPerDollar);
    barClose = ((chartBar[a].close - displayPriceRangeLow) * pixelsPerDollar);

    if (chartBar[a].open > 1)
    {


    COLORREF myColor;
    int xSpacing = 10;
    int xWidth = 5;
    int xMiddle = 2;
    int chartWidth = 400; // 60 on right side for moving close price, 39 bars displayed at once
    int chartHeight = 300; // 15 on top and 15 on bottom
    int chartBottom = 315; // 315, 15 is the top
    int bodyTop; // top of the body
    int bodyBottom; // bottom of the body


    // items are the amounts up from the bottom
    // get correct color for bar and get body top and bottom
    if (barOpen < barClose) // up bar
    {
    myColor = greenColor;

    bodyBottom = barOpen;
    bodyTop = barClose;
    }

    if (barOpen > barClose) // down bar
    {
    myColor = redColor;

    bodyBottom = barClose;
    bodyTop = barOpen;
    }

    if (barOpen == barClose) // same price bar
    {
    myColor = yellowColor;

    bodyBottom = barClose;
    bodyTop = barClose;
    }










    // chart veiwing area is 200 high by 400 long
    // show bars as 5 wide with 5 space inbetween them


    CBrush* mySolidBrush = new CBrush();
    mySolidBrush->CreateSolidBrush(myColor);
    //oldBrush = myDC->SelectObject(mySolidBrush);
    myDC->SelectObject(mySolidBrush);

    // body
    myDC->FillRect(new CRect ((barPosition * xSpacing),(chartBottom - barOpen),((barPosition * xSpacing) + xWidth),(chartBottom - barClose)), mySolidBrush);

    DeleteObject(mySolidBrush);

    myDC->SelectObject(oldBrush);


    CPen* myThinPen = new CPen();//(PS_SOLID(1, myColor));
    myThinPen->CreatePen(PS_SOLID,1, myColor);
    //oldPen = myDC->SelectObject(&myThinPen);
    myDC->SelectObject(myThinPen);

    // upper shadow
    myDC->MoveTo(((barPosition * xSpacing) + xMiddle), (chartBottom - barHigh));
    myDC->LineTo(((barPosition * xSpacing) + xMiddle), (chartBottom - bodyTop));

    // lower shadow
    myDC->MoveTo(((barPosition * xSpacing) + xMiddle), (chartBottom - barLow));
    myDC->LineTo(((barPosition * xSpacing) + xMiddle), (chartBottom - bodyBottom));

    DeleteObject(myThinPen);

    myDC->SelectObject(oldPen);



    }

    }

    DeleteObject(myDC);

    DeleteObject(oldBrush);

    DeleteObject(oldPen);

    //ReleaseDC(myDC);



    dayBarCount = dayBarCount + 1; // bars built so far this session

    } // end void MainDlg::SS_UpdateChart()

  2. #2
    GCDEF is offline Elite Member Power Poster
    Join Date
    Nov 2003
    Location
    Florida
    Posts
    12,635

    Re: crashes after displaying 100 bars??? help please.

    No idea from looking at the code. What does your debugger say?

    A couple of things though.

    1) please learn to use code tags

    2)
    Code:
    if ((displayPriceRangeLow < 1) || (displayPriceRangeLow == NULL))
    {
    displayPriceRangeHigh = 1;
    displayPriceRangeLow = 1;
    }
    // get pixels per dollar
    pixelsPerDollar = displayChartHeight / (displayPriceRangeHigh - displayPriceRangeLow);
    You have potential division by zero there.

    Also, typically NULL is used to test pointer values, not POD types. And since your first test is < 1, NULL would also be < 1, so even if testing for NULL was valid, it would be redundant.

  3. #3
    Join Date
    Apr 2013
    Posts
    4

    Re: crashes after displaying 100 bars??? help please.

    Thanks, I totally missed that division by zero issue there. When testing I use a constant and know that that is not the issue however. I will try this repost with the code tags, great suggestion. The code for drawing the bars or clearing the screen seems to be my trouble - I am not sure that I am getting the resources deleted properly or even setting them up properly. This part:


    [code c++]

    // clear the display for the new bars
    CBrush* blackBrush = new CBrush;

    blackBrush->CreateSolidBrush(blackColor);

    //oldBrush = myDC->SelectObject(blackBrush);
    myDC->SelectObject(blackBrush);

    myDC->FillRect(new CRect (0, 0, 460, 330), blackBrush);

    DeleteObject(blackBrush);

    myDC->SelectObject(oldBrush);


    // find colors to display for each bar space 1 column at a time
    // divide the 100% into the increments of the display

    int barPosition;
    int barOpen;
    int barHigh;
    int barLow;
    int barClose;


    for( int a = 1; a <= 39; a = a + 1)
    {

    barPosition = a;

    barOpen = ((chartBar[a].open - displayPriceRangeLow) * pixelsPerDollar);
    barHigh = ((chartBar[a].high - displayPriceRangeLow) * pixelsPerDollar);
    barLow = ((chartBar[a].low - displayPriceRangeLow) * pixelsPerDollar);
    barClose = ((chartBar[a].close - displayPriceRangeLow) * pixelsPerDollar);

    if (chartBar[a].open > 1)
    {


    COLORREF myColor;
    int xSpacing = 10;
    int xWidth = 5;
    int xMiddle = 2;
    int chartWidth = 400; // 60 on right side for moving close price, 39 bars displayed at once
    int chartHeight = 300; // 15 on top and 15 on bottom
    int chartBottom = 315; // 315, 15 is the top
    int bodyTop; // top of the body
    int bodyBottom; // bottom of the body


    // items are the amounts up from the bottom
    // get correct color for bar and get body top and bottom
    if (barOpen < barClose) // up bar
    {
    myColor = greenColor;

    bodyBottom = barOpen;
    bodyTop = barClose;
    }

    if (barOpen > barClose) // down bar
    {
    myColor = redColor;

    bodyBottom = barClose;
    bodyTop = barOpen;
    }

    if (barOpen == barClose) // same price bar
    {
    myColor = yellowColor;

    bodyBottom = barClose;
    bodyTop = barClose;
    }










    // chart veiwing area is 200 high by 400 long
    // show bars as 5 wide with 5 space inbetween them


    CBrush* mySolidBrush = new CBrush();
    mySolidBrush->CreateSolidBrush(myColor);
    //oldBrush = myDC->SelectObject(mySolidBrush);
    myDC->SelectObject(mySolidBrush);

    // body
    myDC->FillRect(new CRect ((barPosition * xSpacing),(chartBottom - barOpen),((barPosition * xSpacing) + xWidth),(chartBottom - barClose)), mySolidBrush);

    DeleteObject(mySolidBrush);

    myDC->SelectObject(oldBrush);


    CPen* myThinPen = new CPen();//(PS_SOLID(1, myColor));
    myThinPen->CreatePen(PS_SOLID,1, myColor);
    //oldPen = myDC->SelectObject(&myThinPen);
    myDC->SelectObject(myThinPen);

    // upper shadow
    myDC->MoveTo(((barPosition * xSpacing) + xMiddle), (chartBottom - barHigh));
    myDC->LineTo(((barPosition * xSpacing) + xMiddle), (chartBottom - bodyTop));

    // lower shadow
    myDC->MoveTo(((barPosition * xSpacing) + xMiddle), (chartBottom - barLow));
    myDC->LineTo(((barPosition * xSpacing) + xMiddle), (chartBottom - bodyBottom));

    DeleteObject(myThinPen);

    myDC->SelectObject(oldPen);



    }

    }

    DeleteObject(myDC);

    DeleteObject(oldBrush);

    DeleteObject(oldPen);

    //ReleaseDC(myDC);



    dayBarCount = dayBarCount + 1; // bars built so far this session

    } // end void MainDlg::SS_UpdateChart()

    [/code]

  4. #4
    Join Date
    Apr 1999
    Posts
    27,449

    Re: crashes after displaying 100 bars??? help please.

    Quote Originally Posted by numbertea View Post
    Thanks, I totally missed that division by zero issue there. When testing I use a constant and know that that is not the issue however. I will try this repost with the code tags, great suggestion.
    Code tags are done this way:

    [code]
    Your code goes here:
    [/code]


    Regards,

    Paul McKenzie

  5. #5
    Join Date
    Apr 1999
    Posts
    27,449

    Re: crashes after displaying 100 bars??? help please.

    Code:
    for( int a = 1; a <= 39; a = a + 1)
    //...
    barOpen = ((chartBar[a].open - displayPriceRangeLow) * pixelsPerDollar);
    Arrays start at 0 in C++, not 1. Maybe it's your way of accessing an array is what is causing the problem.

    What are the dimensions of your array?

    Regards,

    Paul McKenzie

  6. #6
    Join Date
    Apr 2013
    Posts
    4

    Re: crashes after displaying 100 bars??? help please.

    Yes, I realize the arrays start at 0. For ease of visualizing the pixels myself I declare the arrays with one extra index so that I can start with index 1 instead of zero. Am I correct in this call?

    Code:
    // clear the display for the new bars
     CBrush* blackBrush = new CBrush;
    
     blackBrush->CreateSolidBrush(blackColor);
    
     //oldBrush = myDC->SelectObject(blackBrush);
     myDC->SelectObject(blackBrush);
    
     myDC->FillRect(new CRect (0, 0, 460, 330), blackBrush);
    
     DeleteObject(blackBrush);
    
     myDC->SelectObject(oldBrush);
    Or am I missing something obvious that could be causing a memory leak. Thanks for your reply.

  7. #7
    Join Date
    Apr 1999
    Posts
    27,449

    Re: crashes after displaying 100 bars??? help please.

    Quote Originally Posted by numbertea View Post
    Yes, I realize the arrays start at 0. For ease of visualizing the pixels myself I declare the arrays with one extra index so that I can start with index 1 instead of zero. Am I correct in this call?
    This "technique" almost always leads to an off-by-one error somewhere in the program, causing the program to either crash, invalid data being used, or some other mistake due to accessing an invalid element in the array. I've even seen inconsistent usage of 0 being the start of the data in the array, and the same program somewhere else assuming that 1 is the start of the data.

    Bottom line is this -- arrays start at 0, and writing "unencapsulated" code that tries to fool C++ into thinking that arrays are 1-based is a prime reason for unexpected behaviour to occur. Unless you write a well-tested C++ class that does 1-based indexing, then your loop will always be suspicious.

    You say your program crashes -- that means that any part of the program could have caused this, and not just the code you are showing. How about showing us your array declaration (which I asked for but you did not provide)?

    Regards,

    Paul McKenzie
    Last edited by Paul McKenzie; April 28th, 2013 at 04:12 AM.

  8. #8
    VictorN's Avatar
    VictorN is offline Super Moderator Power Poster
    Join Date
    Jan 2003
    Location
    Hanover Germany
    Posts
    20,396

    Re: crashes after displaying 100 bars??? help please.

    Quote Originally Posted by numbertea View Post
    Code:
    // clear the display for the new bars
     CBrush* blackBrush = new CBrush;
     ...
    Or am I missing something obvious that could be causing a memory leak. Thanks for your reply.
    Well, where do you delete the CBrush* blackBrush object?
    Victor Nijegorodov

  9. #9
    Join Date
    Jan 2002
    Location
    Houston, TX
    Posts
    1,421

    Re: crashes after displaying 100 bars??? help please.

    Quote Originally Posted by numbertea View Post
    Code:
    // clear the display for the new bars
     CBrush* blackBrush = new CBrush;
    
     blackBrush->CreateSolidBrush(blackColor);
    
     //oldBrush = myDC->SelectObject(blackBrush);
     myDC->SelectObject(blackBrush);
    
     myDC->FillRect(new CRect (0, 0, 460, 330), blackBrush);
    
     DeleteObject(blackBrush);
    
     myDC->SelectObject(oldBrush);
    First, as I recall, you cannot delete the object (blackbrush) while it is selected into a DC. You need to reverse the order of the last two lines.

    Also, as mentioned earlier, I don't see where you delete the CBrush (blackbrush), so you have a memory leak.

    Hope this helps...good luck.
    Be sure to rate those who help!
    -------------------------------------------------------------
    Karl - WK5M
    PP-ASEL-IA (N43CS)
    PGP Key: 0xDB02E193
    PGP Key Fingerprint: 8F06 5A2E 2735 892B 821C 871A 0411 94EA DB02 E193

  10. #10
    Join Date
    Apr 2013
    Posts
    4

    Re: crashes after displaying 100 bars??? help please.

    Thanks for your help. I cleaned things up a little and decided to use stock objects and just change the colors. This way a memory leak is not even possible. Realizing it is a bad programming fopa, I kept my arrays for graphics starting at index '1'. I access the array many times in the program and so it just makes things simpler and lowers execution cycles necessary per loop.

    Cheers!

  11. #11
    GCDEF is offline Elite Member Power Poster
    Join Date
    Nov 2003
    Location
    Florida
    Posts
    12,635

    Re: crashes after displaying 100 bars??? help please.

    Quote Originally Posted by numbertea View Post
    Thanks for your help. I cleaned things up a little and decided to use stock objects and just change the colors. This way a memory leak is not even possible. Realizing it is a bad programming fopa, I kept my arrays for graphics starting at index '1'. I access the array many times in the program and so it just makes things simpler and lowers execution cycles necessary per loop.

    Cheers!
    That's a horrible idea and doesn't make things simpler and I don't see how it lowers execution cycles. It'll be a maintenance nightmare. Get out of that habit immediately.

  12. #12
    Join Date
    Apr 1999
    Posts
    27,449

    Re: crashes after displaying 100 bars??? help please.

    Quote Originally Posted by numbertea View Post
    I kept my arrays for graphics starting at index '1'. I access the array many times in the program and so it just makes things simpler and lowers execution cycles necessary per loop.
    ????

    If you loop x times, you loop x times. It doesn't matter what the lower and upper bounds are if (upper_bound - lower_bound + 1) == x. For example, there is no difference in the number of loops done if you go from 0 to 9 or from 1 to 10. The loop is performed 10 times.

    What I'm saying is that it is dangerous to use a lower bound that is not 0 for array, since arrays start at 0. Going off the end of the array, or erroneously accessing element 0 is one of the most hard to detect bugs, since many times you never know you've gone over the end until you need to run that program somewhere else (usually at a customer site, where your reputation is on the line). You either get a crash, or in the case of accessing element 0, you get bad or uninitialized data being used.

    So it doesn't matter if you ran the program thousands of times on your machine seemingly without problems -- any code where you fool C++ into thinking that arrays start at 1 instead of 0 is always suspicious, especially if you're experiencing crashes. The only way to remove that doubt is to either treat your arrays correctly (start from 0), or write a class that encapsulates handling 1-based arrays, and use that class.

    I have seen tons of code where the coder is doing something similar to your code with the 1-based arrays, and in almost all of the times I've seen the code, there is a bug somewhere in the accessing of the arrays. If not a bug, the code becomes a maintenance headache for anyone attempting to change the code, or even the original coder loses track and introduces bugs.

    Also, you fix a crash by actually fixing the crash. By just changing code and seeing that the crash doesn't happen anymore is not a fix. All you did was rearrange the code without addressing the real problem, and that does not guarantee you fixed anything.

    You need to identify why the crash occurs, and then create a fix that addresses the "why".

    Regards,

    Paul McKenzie
    Last edited by Paul McKenzie; May 1st, 2013 at 10:26 AM.

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •  





Click Here to Expand Forum to Full Width

Featured