[RESOLVED] memcpy error in MFC

Hi,

I load an image(768*256) using a file path(OpenImageFilePath).

After load an image, I start to read the loaded Image data using GetBits method and plot the same image data(768*256).

I'm using the memcpy method, for that i'm getting the below error. memmove function also giving the same error message.

Please clear me.

File Name : memcopy.asm
rep movsd ;N - move all of our dwords

Code for your reference

Code:

---

void CDlg :: FileOpen()
{
        CFileException CFileEx;
        CStdioFile ReadFile;

        // szFilters is a text string that includes two file name filters:
        TCHAR szFilters[]= _T("Image Files (*.bmp)");

        CFileDialog fileDlg(TRUE, _T("bmp"), _T("*.bmp"),

        OFN_FILEMUSTEXIST | OFN_HIDEREADONLY, szFilters);

        fileDlg.m_ofn.lpstrTitle = _T("Offline Images");

        fileDlg.m_pOFN->lpstrInitialDir=_T("C:\\AcceptedImg\\");

        if(fileDlg.DoModal() == IDOK)
        {
                OpenImageFilePath = fileDlg.GetPathName();
                OpenImageFileFlag = true;
        }

        //Refresh Window
        CRect mRect;
        GetClientRect(&mRect);
        InvalidateRect(&mRect);
}

void CDlg::OnPaint()
{
        CPaintDC dc(this); // device context for painting
        if(OpenImageFileFlag)
        {        //Load Image
                CDC *pDC;
                pDC=GetDC();

                CImage ObjImage ;
                ObjImage.Load(OpenImageFilePath) ; //Image Path

                CBitmap pBitmap ;
                pBitmap.Attach(ObjImage.Detach());

                BITMAP bm;
                pBitmap.GetObject( sizeof(BITMAP), &bm );       

                CDC dcMem;
                dcMem.CreateCompatibleDC( pDC );

                CBitmap* pbmpOld = dcMem.SelectObject( &pBitmap );
                pDC->BitBlt(25,360, bm.bmWidth, bm.bmHeight,&dcMem, 0,0, SRCCOPY );

                dcMem.SelectObject( pbmpOld );
                ReleaseDC( pDC );

                //Get Bits from that image
                CImage atlImage;
                atlImage.Load(OpenImageFilePath);

                void* pPixel = NULL;
                pPixel  = atlImage.GetBits();

                int pitch = atlImage.GetPitch();
                int bytes = abs(pitch) * atlImage.GetHeight();

                const BYTE * src = NULL;
                src = (BYTE*)atlImage.GetBits();

                if(pitch < 0) src -= bytes;

                BYTE * pBitmapData = NULL;
                pBitmapData = new BYTE[bytes];

                memcpy(pBitmapData, src, bytes);
                //memmove(pBitmapData, src, bytes); //Getting Error foe use memmove func also

                width = atlImage.GetWidth();       
                height = atlImage.GetHeight();
               
                for (int y = 0; y < width; y = y + 1)
                {
                        iIndex = y + (width * height) - width;
                        for (int x = 0; x < height ; x = x + 1)
                        {
                                OfflineImageData[iIndex] = (byte)(*(pBitmapData + iIndex));
                                OfflineData[y][x] = OfflineImageData[iIndex];
                                iIndex = iIndex - width;
                        }
                }
        }//End OpenImageFileFlag
        for (int y = 0; y < width; y = y + 1)//1024 ImageHeight
        {
                for (int x = 0; x < height-1 ; x = x + 1)//256 ImageWidth
                {
                        SetPixelV(dc,25+y,50+x,RGB(OfflineData[y][x],OfflineData[y][x],OfflineData[y][x]));
                }
        }
}

---

memcpy usually only causes problems when either the source or destination pointers are invalid or you exceed the buffer size.

Set a breakpoint on the memcpy line and verify each parameter has the values you expect.

You do not provide a compilable project, and from your description it's not clear what kind of error you get. Is it compile time error? run time error? If run time, did you try to debug your code and inspect the parameters that were passed to memcpy?

As for the WM_PAIN handler design, it seems you totally ignored all my comments provided in the other thread's post. So please be aware that you keep going wrong way.

Quote:

---

Originally Posted by Arjay

memcpy usually only causes problems when either the source or destination pointers are invalid or you exceed the buffer size.

---

I Accept this. But I'm load 768&256 data & plot the same buffer size data only.

Quote:

---

Originally Posted by Igor Vartanov

You do not provide a compilable project.

---

Kindly find the attachment. Its a run time error.

I'm getting 2 types of error.

1. If i double click the OpenDlgFile path , then I'm getting Reading Vialation error. (Not all time. some time only getting this error)

2. I'm getting 768 * 256 line data. But If i plot the data means, display contains 767 * 255 Only(View).(Line data missing)

Kindly find the image files for your reference.

Please clear me. Am i going right way?

So what's the result of Arjay's request from post #2?

Quote:

---

Originally Posted by Arjay

memcpy usually only causes problems when either the source or destination pointers are invalid or you exceed the buffer size.

---

I find my mistake and cleared. Its a exceed buffer size problem.

Code:

---

CImage atlImage; CBitmap pBitmap1 ; BITMAP bm1;

atlImage.Load(OpenImageFilePath);

pBitmap1.Attach(atlImage.Detach());

pBitmap1.GetObject( sizeof(BITMAP), &bm1 );

width = bm1.bmWidth;    height = bm1.bmHeight;

int pitch = width;

int bytes = abs(pitch) * height;

BYTE * src = new BYTE[bytes];

src = (BYTE*)bm1.bmBits;

if(pitch < 0) src -= bytes;

BYTE * pBitmapData = NULL;

pBitmapData = new BYTE[bytes];

memcpy(pBitmapData, src, bytes);

---

Am i correct?

Quote:

---

Originally Posted by saraswathisrinath

I find my mistake and cleared. Its a exceed buffer size problem.

---

Code:

---

BYTE * src = new BYTE[bytes];
src = (BYTE*)bm1.bmBits;

---

Do you see what's wrong with those two lines? Forget about bitmap data for a moment -- on a C++ level, do you see the issue with those two lines above?

The new[] returns a pointer value. Then on the second line, you just throw it away and replace that value with another. How are you going to deallocate the memory if you throw away the original value?

Your code is no different than just doing this:

Code:

---

src = (BYTE*)bm1.bmBits;

---

but it's worse now that you've introduced a memory leak.

Regards,

Paul McKenzie

I'm in beginner stage. If i'm doing any mistake, please take me to a right path at all time.

I will replace my code. Thanks Mr.Paul

Quote:

---

Originally Posted by saraswathisrinath

Code:

---

CImage atlImage; CBitmap pBitmap1 ; BITMAP bm1;
atlImage.Load(OpenImageFilePath);
pBitmap1.Attach(atlImage.Detach());
pBitmap1.GetObject( sizeof(BITMAP), &bm1 );
width = bm1.bmWidth;    height = bm1.bmHeight;
int pitch = width;
int bytes = abs(pitch) * height;
BYTE * src = new BYTE[bytes];
src = (BYTE*)bm1.bmBits;
if(pitch < 0) src -= bytes;
BYTE * pBitmapData = NULL;
pBitmapData = new BYTE[bytes];
memcpy(pBitmapData, src, bytes);

---

---

Please, do not put more than one statement in the same line! Code written in such a manner (two or three statements per line) is very hard to read, very hard to debug and very hard to maintain!
Besides, why do you insert an empty line after each line with code? It only makes sense if you separate some logical parts or blocks of your code, but there is no any in your code snippet!

Quote:

---

Originally Posted by saraswathisrinath

I'm in beginner stage. If i'm doing any mistake, please take me to a right path at all time.

---

Code:

---

int main()
{
  int x = 0;
  int *p = new int [10];
  p = &x;
}

---

This is basically what your bit of code I pointed out is doing. You tell us -- do you see what's wrong with those three lines of code above? It gets worse if you then do this:

Code:

---

int main()
{
  int x = 0;
  int *p = new int [10];
  p = &x;
  delete [] p;
}

---

What is wrong with the lines above? If you don't know, then you have a much larger issue than bitmap handling -- you need to know how to use the C++ language properly.

Regards,

Paul McKenzie

Quote:

---

Originally Posted by Paul McKenzie

[code]
int main()
{
int x = 0;
int *p = new int [10];
p = &x;
}
[/code

---

i understood like, pointer p allocate the memory using new operator. So we must use the delete operator to deallocate.
But here,

Code:

---

p = &x;  // declaration is wrong - reference operator &  can't allow to deallocate the memory

---

we can use like below only,

Code:

---

int * p = new int[10];
(or)
int x = 10;
int *p = new int [x];
(or)
int x = 0;
int * p = &x; // Point to address of the veriable

---

Quote:

---

Originally Posted by Paul McKenzie

Code:

---

int main()
{
  int x = 0;
  int *p = new int [10];
  p = &x;
  delete [] p;
}

---

---

here, we can use like below

Code:

---

int x = 10;
int *p = new int [x];
//p = &x;    //wrong - we can't use delete operator
delete [] p;

---

am i correct?

Quote:

---

Originally Posted by saraswathisrinath

i understood like, pointer p allocate the memory using new operator. So we must use the delete operator to deallocate.
But here,

Code:

---

p = &x;  // declaration is wrong - reference operator &  can't allow to deallocate the memory

---

---

The "&" used above is not the reference operator. That is the address-of operator, and has nothing to do with references. The code is perfectly valid. There is nothing wrong syntactically with it.

What is wrong is that the pointer value returned by operator new[] is stored in p, but then it is thrown away by assigning another address to "p" immediately afterwards. That is exactly what your code you say is "resolved" was doing, so nothing has been "resolved".

All you did in your new code was to move the memory corruption bug somewhere else. These lines did absolutely nothing except cause a memory leak, and worse, it masked the underlying problem:

Code:

---

BYTE * src = new BYTE[bytes];  // allocating memory, storing pointer value to allocated memory in "src"
src = (BYTE*)bm1.bmBits;    // throwing away pointer value returned by new[] above with another pointer value.  This is wrong!

---

Do you see the similarity between the code above, and the simple example I posted?

What I suggest is that you get rid of your "solution", bring back the old code that caused the problem, and fix the program correctly without throwing random lines of code that just masks the bug (i.e. changes the executable image so that the memory bug is hidden, but not fixed).

Unlike other computer languages, it is very easy to think that a problem is "resolved" in C++ by writing random lines of code and seeing the problem seemingly disappear without reason. But that is not a solution -- the only coded solutions in C++ are ones that can be explained coherently as to why the new code solves the problem.

Regards,

Paul McKenzie

Quote:

---

Originally Posted by Paul McKenzie

Unlike other computer languages, it is very easy to think that a problem is "resolved" in C++ by writing random lines of code and seeing the problem seemingly disappear without reason. But that is not a solution -- the only coded solutions in C++ are ones that can be explained coherently as to why the new code solves the problem.

---

True, so true. When trying to find a problem in a program the one thing I hate above all others is to add an innocuous print statement and either find the problem 'has gone away' or a different problem appears. This invariably means a memory corruption problem somwhere and a late night debugging session! Just because a c++ program seems to work on your computer doesn't mean that it will work correctly on another computer - or even on yours the next day! That's why c++ programs need to be carefully designed, coded, tested and thoroughly debugged to make sure they are robust.

Quote:

---

Originally Posted by 2kaud

That's why c++ programs need to be carefully designed, coded, tested and thoroughly debugged to make sure they are robust.

---

You make it sound like C++ is necessarily so.
You can just as well adopt methodology and use/by/develop libraries that will let you develop C++ code with the same ease as other languages while still allowing you to go "out of the confines of the box" if such is needed.

Yes, C++ allows you to write "messy" code, that doesn't mean it's smart to do so :)

Quote:

---

Originally Posted by OReubens

You can just as well adopt methodology and use/by/develop libraries that will let you develop C++ code with the same ease as other languages while still allowing you to go "out of the confines of the box" if such is needed.

Yes, C++ allows you to write "messy" code, that doesn't mean it's smart to do so

---

Agreed. But IMO this comes back to a recurring theme in these forums - correct teaching of c++.

I realized. Thanks MR.Paul & all gurus.