Why is the exception thrown on memcpy using during copying LPBYTE to LPTSTR (clipboard)?

Question

Hello, I have a LPBYTE array (taken from file) and I need to copy it into LPTSRT (actually into the clipboard). The trouble is copying work but unstable, sometime an exception was thrown (not always) and I don't understand why. The code is:

     FILE *fConnect = _wfopen(connectFilePath, _T("rb"));
  if (!fConnect)
   return;
  fseek(fConnect, 0, SEEK_END);
  lSize = ftell(fConnect);
  rewind(fConnect);

  LPBYTE lpByte = (LPBYTE) malloc(lSize);  
  fread(lpByte, 1, lSize, fConnect); 
  lpByte[lSize] = 0;
  fclose(fConnect);

  //Copy into clipboard
  BOOL openRes = OpenClipboard(NULL);
  if (!openRes)
   return;
  DWORD err = GetLastError();

  EmptyClipboard(); 
  HGLOBAL hText;
  hText = GlobalAlloc(GMEM_MOVEABLE, (lSize+ sizeof(TCHAR)));

  LPTSTR sMem = (TCHAR*)GlobalLock(hText); 
  memcpy(sMem, lpByte, (lSize + sizeof(TCHAR)));

The last string is the place where the exception is thrown. Thanks a lot

Answer 1

Do GlobalAlloc or GlobalLock work? Put some error checking code in and see, both should return non-NULL values.

Answer 2

I'm not saying, it's the cause of Your problems, but it may be or may be a cause of other problems in the future.

If You allocate memory like this

LPBYTE lpByte = (LPBYTE) malloc(lSize);  

This is an access outside of the allocated chunk of memory:

lpByte[lSize] = 0;

Allocated memory has size of lSize and it contains locations form &lpByte[0] to &lpByte[lSize - 1] inclusive.

EDIT:

As Hans noticed, memcpy also accesses the memory outside of the allocated block. If sizeof(TCHAR) is 1, the last read byte is lpByte[lSize] and if sizeof(TCHAR) is more that 1, bytes past lpByte[lSize] are also read or at least attempted to be.

Answer 3

I am not sure about what causes problems in your code, but the following code works and everything is locked / copied fine (note that your clipboard operations could be easily commented out and have no impact on the problem's source):

   LPBYTE lpByte = (LPBYTE)malloc(512);  
   lpByte[0] = 'A';
   lpByte[1] = 'B';
   lpByte[2] = '0';

   // OpenClipboard(NULL);
   // EmptyClipboard(); 

   HGLOBAL hText;
   hText = GlobalAlloc(GMEM_MOVEABLE, 512);

   LPTSTR sMem = (TCHAR*)GlobalLock(hText); 
   memcpy(sMem, lpByte, 512);

You could try setting breakpoints in your code right before the exception happens (it could actually have different reasons).

Answer 4

_wfopen is the wide character version of fopen - you should be passing it L"...", not TCHARs. The TCHAR version is _tfopen (which will boil down to one of fopen or _wfopen) - See: http://msdn.microsoft.com/en-us/library/yeby3zcb%28VS.80%29.aspx

LPBYTE lpByte = (LPBYTE) malloc(lSize);

If this is C, you don't really need to cast malloc's result. Personally, the MS LP* types leave a bad taste in my mouth - I feel the Hungarian obscures the readability of the code to someone well versed in... C. Thus, I prefer BYTE * over LPBYTE, but it's not going to make or break code.

fread(lpByte, 1, lSize, fConnect);

Check the return value.

lpByte[lSize] = 0;

As others mentioned, this memory access is out of the array bounds.

if (!openRes)
    return;
DWORD err = GetLastError();

• You leak lpByte
• You call GetLastError() ... on success?

Next,

LPTSTR sMem = (TCHAR*)GlobalLock(hText);

While I prefer the non-LP stuff, perhaps choose one? (Maybe make the cast LPTSTR?) Again, it shouldn't matter in the end. (This might fall under a "It's a malloc, and doesn't need a cast" as well.)

memcpy(sMem, lpByte, (lSize + sizeof(TCHAR)));

As others mentioned, this memcpy is also accessing invalid memory. Specifically, lpByte is lSize long, but you're doing that plus an extra sizeof(TCHAR).