C++ Pointer/Heap Problem

Question

4.50/5 (2 votes)

See more:

Hey, I'm struggling against the following problem. I wrote a simple gaus algorithm, but AFTER it is done executing (succesfuly), pretty much any next attempt to alocate memory crashes the program and gives me the following error:

"Windows has triggetred a breakpoint in ...
This may be due to a corruption of heap, which indicates...

and displays malloc.c to me.

Any ideas what im doing wrong?

Thanks in advance

The code:

C#

#ifndef __GAUS_H_INCLUDED__
#define __GAUS_H_INCLUDED__
#include "Grid.h"
#include <math.h>

class Gaus
{
private:


public:

Gaus ()
{
}
Gaus ( GPA * Source, GPA * Result)
{
CallGaus(Source,Result);
}

void CallGaus( GPA * Source, GPA * Result )
{
  int xa, xe, ya, ye, sx, sy=0, T, s; //Bildcoords-Laufvariablen
  Punkt P;
   s=(*Source).X;
  Punkt*   R1;
  Punkt*   R2;
  Punkt*   R3;
  R1 = new Punkt[s];
  R2 = new Punkt[s];
  R3 = new Punkt[s];

xa=0; ya=0; xe=(*Source).X; ye=(*Source).Y;


  sy=ya;
  for (sx = xa; sx < xe; sx++)                              // Zeile 1
  { P=(*Source)[sx][sy]; R2[sx]=P; }
  for (sx = xa; sx < xe; sx++)                              // Zeile 2
  { P=(*Source)[sx][sy+1]; R3[sx]=P; }

   for (sy=ya+1; sy < ye-1; sy++) {
       int sy1=sy+1;
       int xa1=xa+1;
       R1=R2;
       R2=R3;
       R3[sx]=(*Source)[xa][sy1];
       R3[sx+1]=(*Source)[xa1][sy1];
       int r2=int(R1[xa].R+2*R2[xa].R+R3[xa].R);
       int G2=int(R1[xa].G+2*R2[xa].G+R3[xa].G);
       int B2=int(R1[xa].B+2*R2[xa].B+R3[xa].B);
       int r3=int(R1[xa1].R+2*R2[xa1].R+R3[xa1].R);
       int G3=int(R1[xa1].G+2*R2[xa1].G+R3[xa1].G);
       int B3=int(R1[xa1].B+2*R2[xa1].B+R3[xa1].B);
   for (T=(xa-1);T<(xe-1);T++)
   {
       int T1=T+1;
       R3[T1]=(*Source)[T1][sy1];
       int r1=r2;
       int G1=G2;
       int B1=B2;
       r2=r3; G2=G3; B2=B3;
       r3=int(R1[T1].R+2*R2[T1].R+R3[T1].R);
       G3=int(R1[T1].G+2*R2[T1].G+R3[T1].G);
       B3=int(R1[T1].B+2*R2[T1].B+R3[T1].B);
       int rr=(r1+2*r2+r3)/16;
       int gg=(G1+2*G2+G3)/16;
       int bb=(B1+2*B2+B3)/16;
       P.R=rr;
       P.G=gg;
       P.B=bb;

       (*Result).WriteSecure(T, sy, &P);
   }
  }



}
~Gaus()
    {

    }

};
#endif

Posted 13-Jun-12 12:32pm

Y0UR

Updated 13-Jun-12 12:35pm

v2

Add a Solution

2 solutions

Solution 1

You're overriding the bounds of one of the dynamically allocated arrays, use your debugger to find out which one by simply stepping through your loops...or, you can look at the call stack to see which call triggered the error.

Edit: I also see you're allocating memory and never deallocating it... Do you like making memory leaks?

Posted 13-Jun-12 12:58pm

Albert Holguin

Updated 13-Jun-12 13:01pm

v2

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Aescleal · Accepted Answer · 2012-06-13T18:57:00

You're doing pointer manipulation on pointers to dynamically allocated memory. This has a couple of effects:

- you're orphaning blocks of memory - you couldn't free R1, even if you wanted to
- as Albert said you're overrunning an array somewhere. You're not assigning xa anywhere for one thing so it's left with whatever was in that memory location before it was reallocated to hold your variable

So, what can you do about this lot?

- use std::vector instead of fiddling about with dynamic memory. This will sort your memory leaks out, cause even if you assign them. This includes the input parameters - use a const reference for Source and a reference for Result. This also includes the parts of GPA that are arrays (if they're not just arrays of arrays of points)

- Lean on the runtime library. If you're using VC++ in debug mode you'll get some sort of assert of exception if you run over the bounds of a vector. If you don't there might be a way to turn on bounds checking for your compiler. If there isn't do a assert that array indices are valid. Some compilers do iterator bounds checking so perhaps use iterators rather than the [] operator - it'll make your code a bit uglier but easier for the runtime library to catch errors

Some other comments...

- don't use redundant variables where you don't need to, e.g. xa and ya aren't assigned to. xa1 and sy1 are just aliases for xa + 1, sy +1, T1 for T + 1 xe and s for Source->X and ye for Source->Y. sy is always zero during your first two loops. sx starts at 0 the first two loops
- always initialise variables
- if you have to cast use C++ casts. If you find you're reinterpret_casting all over the shop there is something wrong with what you're doing
- use a->b rather than (*a).b. It's a character less and is a bit more obvious to jaded old hacks like me
- why's Gaus a class? Looks like a function to me. There's no state and the constructors/destructors don't do anything. It can't even be used as a functor as it doesn't have an overloaded operator()
- where's T declared?? Don't use global variables! Ever. For anything
- Look at the lines R1=R2; R2=R3. First iteration R1 = R2(intial) and R2=R3(initial). Second and subsequent iterations R1=R3(initial) and R2=R3(intial). Are you sure you want that?
- ~~same sort of comment for int r1=r2; int G1=G2; int B1=B2; r2=r3; G2=G3; B2=B3;~~
- scratch that, was too tired to see r3, G3 and B3 were modified later in the same iteration
- don't use declare loop variables at a larger scope than they need to be. All of yours are.
- don't declare variables until you need them. P for instance has way too much scope and looks like it could do with a three parameter constructor
- Source and Result's declarations look a bit suspect to me - especially as *Source and *Result are 2D arrays.
- are you sure you need P? If I'm reading right a three term constructor would be enough and where you pass &P into WriteSecure a const reference will do

Get rid of that lot and it wouldn't surprise me if the error became glaringly obvious!

Jesus, spent 3 hours reviewing that lot - you owe me £150 at my usual training contract rate. £75 for students. Where do I send the invoice? :-)