-1

My function zeroPadding has more options but I put only two because they're important for this example. When I debug the main() function I got this error

Access violation reading location 0xCCCCCCCC

Why?


This is from the class Signal just to show what the constructor calls

template <class T> Signal<T>::Signal(int width,int height){
    N=width; 
    M=height;
    sig2D= new int*[N];
    for(int i=0;i<N;i++){
        sig2D[i]=new int[M];
    }
    t0=0;
    deltaT=1;
    for (int i=0;i<N;i++)
        for (int j=0;j<M;j++){
        sig2D[i][j]=t0+j*deltaT;
    }
}

    template <class T> Image<T>::Image(int width,int height): Signal(width,height) {}

    template <class T> Image<T> Image<T>::zeroPadding(Image<T> im2){

        Signal<T> s1= static_cast<Signal<T>>(*this);
        Signal<T> s2= static_cast<Signal<T>>(im2);   

        int *temp=new int[15];
    for(int i=0;i<15;i++){
        temp[i]=0;
    }

        if(s1.getWidth()>s2.getWidth() && s1.getHeight()==s2.getHeight()){

            im2.setWidth(s1.getWidth());
            im2.sig2D=new T*[im2.getWidth()];
            for(int i=0;i<im2.getWidth();i++){
                im2.sig2D[i]=new T[im2.getHeight()];
            }

            for (int i=s2.getWidth();i<s1.getWidth();i++)
                for(int j=0;j<s2.getHeight();j++){
                    im2.sig2D[i][j]=temp[j];
                }   
                return im2;

        }
          else if(s1.getHeight()<s2.getHeight() && s1.getWidth()>s2.getWidth()){
        setHeight(s2.getHeight());
        sig2D=new T*[getWidth()];
        for(int i=0;i<getWidth();i++){
            sig2D[i]=new T[getHeight()];
        }

        for (int i=0;i<s1.getWidth();i++)
            for(int j=s1.getHeight();j<s2.getHeight();j++){
                sig2D[i][j]=temp[j];
            }   
            (*this).zeroPadding(im2);
            }
    }    

    template <class T> Image<T> Image<T>::addImage(Image<T> im2){
        Image<T> *r=new Image(80,80);


        if ((*this).getWidth()==im2.getWidth() && (*this).getHeight()==im2.getHeight()) {
            //r=new Image(im2.getWidth(),im2.getHeight());
            for (int i=0;i<im2.getWidth();i++){
                for(int j=0;j<im2.getHeight();j++)
                (*r).sig2D[i][j]= (*this).sig2D[i][j]+im2.sig2D[i][j];
            }
            return(*r);
        }
        else {
          (*r)= zeroPadding(im2); // here breaks why?
              addImage(*r); 
        }

    int main() {

        Image<int> *a= new Image<int>(5,6);
        Image<int> *b= new Image<int>(4,7);

        (*a).addImage(*b);
        return 0;
    }
phuclv
  • 37,963
  • 15
  • 156
  • 475
user3094708
  • 1
  • 1
  • 3
  • 5
    In this case `Access violation reading location 0xCCCCCCCC` means you are dereferencing an uninitialized pointer. Use a debugger! Also stop it with the `(*this).` and use `this->` instead. – user703016 Dec 19 '13 at 10:06
  • try valgrind or similar – arne Dec 19 '13 at 10:07
  • 5
    `int temp[]={0};` This array has only one element in it. You are then attempting to access `temp[j]` where `j` has values other than `0`. – Joseph Mansfield Dec 19 '13 at 10:08
  • If it says there's an access violation *reading* a location, the problem shouldn't be with what `r` points to, but something else. –  Dec 19 '13 at 10:08
  • C++ doesn't support implied int like in `N=width; M=height;`. And you should never use it like that – phuclv Dec 19 '13 at 10:39
  • BTW, 0xCCCCCCCC is one of the magical numbers in VC that programmers should know http://stackoverflow.com/questions/370195/when-and-why-will-an-os-initialise-memory-to-0xcd-0xdd-etc-on-malloc-free-new it'll help debugging uninitialized memory much easier – phuclv Dec 19 '13 at 10:42
  • But I still don't understand where I didn't initialize r or something else. – user3094708 Dec 19 '13 at 10:46
  • I when I called (*r)=(*r).zeroPadding(im2) instead of (*r)=zeroPadding(im2) it passed so it's not problem with r, problem is with (*this). I tought that (*this) is initialized in main(), am I wrong? – user3094708 Dec 19 '13 at 12:07
  • use a debugger and find out where the program stopped. And remember to check after allocation. You're creating an array `int *temp=new int[15];`, `sig2D= new int*[N];` and then use it without checking the `new` has completed successfully – phuclv Dec 19 '13 at 12:49
  • I debug in main function in line (*a).addImage(*b) so debugger come to (*r)=zeroPadding(im2); and after that go through zeroPadding and come back to line (*r)=zeroPadding(im2); and I click Step over and breaks. – user3094708 Dec 19 '13 at 13:02
  • Does the class `Signal` have a [copy constructor](http://stackoverflow.com/questions/2168201/what-is-a-copy-constructor-in-c) or a [move constructor](http://en.cppreference.com/w/cpp/language/move_constructor)? If yes, please post them (the bug may be there). – anatolyg Dec 19 '13 at 13:10
  • @user3094708 you didn't show the copy constructor. Maybe you don't have one but you delete sig2D in the destructor? – Henrik Dec 19 '13 at 13:29
  • @LưuVĩnhPhúc - `new` will throw if it fails. No need to check. – Roddy Dec 19 '13 at 13:40
  • I have copy constructor template Signal::Signal(Signal& s):N(s.N),M(s.M),sig2D(s.sig2D) {} .Is this OK? But I have still the same error. – user3094708 Dec 19 '13 at 13:41
  • @user3094708 no, this is not OK. You're creating a shallow copy. When you delete sig2D in one instance, the pointer in the other instance is invalidated too. – Henrik Dec 19 '13 at 13:56
  • I change my copy constructor but still same error. template Signal::Signal(Signal& s):N(s.N),M(s.M){ T** ss= new T*[s.N]; for(int i=0;i – user3094708 Dec 19 '13 at 14:07
  • @Henrkin I don't understand. Can you explain me what do you mean "When you delete sig2D in one instance, the pointer in the other instance is invalidated too" – user3094708 Dec 19 '13 at 14:11

1 Answers1

0

The most serious problem with your code is that you only return in the first if in zeroPadding which cause undefined returning value if the program flows into else.

if(s1.getWidth()>s2.getWidth() && s1.getHeight()==s2.getHeight()){

            im2.setWidth(s1.getWidth());
            im2.sig2D=new T*[im2.getWidth()];
            for(int i=0;i<im2.getWidth();i++){
                im2.sig2D[i]=new T[im2.getHeight()];
            }

            for (int i=s2.getWidth();i<s1.getWidth();i++)
                for(int j=0;j<s2.getHeight();j++){
                    im2.sig2D[i][j]=temp[j];
                }   
                return im2;  // <- return here

        }
          else if(s1.getHeight()<s2.getHeight() && s1.getWidth()>s2.getWidth()){
        // what will it return if the code comes to here
        setHeight(s2.getHeight());
        sig2D=new T*[getWidth()];
        for(int i=0;i<getWidth();i++){
            sig2D[i]=new T[getHeight()];
        }

        for (int i=0;i<s1.getWidth();i++)
            for(int j=s1.getHeight();j<s2.getHeight();j++){
                sig2D[i][j]=temp[j];
            }   
            (*this).zeroPadding(im2);
            }
       // or here

But there are also many problems with your code

First, properties and methods in the same object don't need this, just sig2D[i][j], zeroPadding(im2);... is enough.

You're also generating lots of memory leaks by allocating memory without freeing. In the addImage function you create a new image

Image<T> *r=new Image(80,80);

then in the if you create a new image again without freeing the previous one

r=new Image(im2.getWidth(),im2.getHeight());

Besides, you're using allocated memory without checking for NULL

sig2D= new int*[N];
for(int i=0;i<N;i++){
    sig2D[i]=new int[M];
}

int *temp=new int[15];
for(int i=0;i<15;i++){
    temp[i]=0;

also in the addImage function, if it goes to the else

(*r)= zeroPadding(im2); // here breaks why?

if the new statement fails, the assignment also fails. And in the code you said it failed at the above line while in the comment you said it failed with (*this):

I when I called (*r)=(*r).zeroPadding(im2) instead of (*r)=zeroPadding(im2) it passed so it's not problem with r, problem is with (*this). I tought that (*this) is initialized in main(), am I wrong?

Yes you're wrong. this is just a pointer to the current object, not a real variable, so if the object exists, access to this are always successful. But if r is not initialized then the line in your comment won't continue running.

Using -> instead of (*x). will make your code shorter and cleaner

Last, please refix the indent of your code

phuclv
  • 37,963
  • 15
  • 156
  • 475
  • In every place that I have =new I put it try and catch block (catch is like this catch(std::bad_alloc&){cout << "New failed";} ) and debug from main. Function addImage go inside zeroPadding (*r)=zeroPadding(im2) and return to this line without error. But after come back to this line I put Step over and fail. – user3094708 Dec 19 '13 at 16:05
  • I agree with you that I don't need(*this).zeroPadding. I didn't put whole function zeroPadding because is big so I put only one part which are executed in this example in main. So my zeroPadding in this example start in else if where at the end of else if (*this).zeroPadding(im2) is called. After that first part od zeroPadding is executed (if part). So it's not necessary to have return in else if because at the end of else if I will call again zeroPadding which will execute part of function with return.I put try and catch in every place where I use new I didn't catch error. – user3094708 Dec 19 '13 at 18:41
  • you're assuming wrong. If it was called recursively again then the result will be consumed by the caller, which `(*this).zeroPadding(im2);` was discarded by this line without returning to the outer caller. If you need tail recursion then use `return (*this).zeroPadding(im2);` – phuclv Dec 20 '13 at 00:35
  • imagine `int a(int n) { if (somecondition) return n; else a(n - 1); }`, if control goes to else then nothing will be returned. Turn on all warning and see – phuclv Dec 20 '13 at 00:39
  • @ Lưu Vĩnh Phúc you were rigth. I don't have error right now. Thank you. – user3094708 Dec 20 '13 at 08:51
  • I changed else part of addImage function so instead of (*r)= zeroPadding(im2);addImage(*r); I put return addImage(zeroPadding(im2)); so after this line if part of addImage is called. But It breaks in line (*r).sig2D[i][j]=(*this).sig2D[i][j]+(im2).sig2D[i][j]; when variable i become 3. I don't understand why it breaks when i=3 because in this example i goes from 0 to 4. – user3094708 Dec 20 '13 at 15:03