2

Two problems with the below code. To begin, I have been scouring this and various other forums for answers to my 1784 error code and everything I've tried has failed. Two of the threads I've checked on stackoverflow are WriteFile returning error 1784 and BlockWrite I/O Error 1784. I've checked some others on this forum but I'm not remembering exactly what the are right now.

I'm trying to save an array of structs to an empty binary file. The first problem is that I get an access violation if my size variable (nNumberOfBytesToWrite parameter) is anything less about 99000 bytes. That number jumps around. For awhile when I was testing it would have the access violation if it was 99,999 bytes but not 100,000 bytes. Of course, what I eventually want to do is set the size to the size of the entire array. The original code to handle that is now commented out so I can test with various sizes.

The second thing that happens (if I don't get an access violation) is I get error code 1784 and WriteFile fails every time. As other threads on this topic have stated, this is defined on MSDN as ERROR_INVALID_USER_BUFFER and the description is "The supplied user buffer is not valid for the requested operation." I've looked at MSDN's own example for opening files like this (http://msdn.microsoft.com/en-us/library/windows/desktop/bb540534%28v=vs.85%29.aspx) and have tried some variations based on their code, but nothing seems to work.

This problem is probably massively noob and I'm sure I'm overlooking something ridiculously simple, but if anyone has suggestions they'd be greatly appreciated.

case IDM_SAVE1:
{
    HANDLE hFile = CreateFile("MineSave.mss", GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
    int test_buffer[] = {1,2,3,4,5,6,7,8,9,10};

    if(hFile != INVALID_HANDLE_VALUE)
    {
        BOOL bSuccess;
        DWORD size = 100000; //DWORD size = (((sizeof(tile)) * tiles_total));
        LPDWORD bytes_written = 0;
        bSuccess = WriteFile(hFile, test_buffer, size, bytes_written, NULL);
        if(bSuccess)
        {
            MessageBox(hwnd, "File saved successfully.", "Great Job!", MB_OK);
        }
        else
        {
            DWORD error = GetLastError();
            MessageBox(hwnd, "Could not write to file.", "Error", MB_OK);
        }

        CloseHandle(hFile);
    }
    else
    {
        MessageBox(hwnd, "Could not create file.", "Error", MB_OK);
    }
}
break;
Community
  • 1
  • 1
John White
  • 79
  • 2
  • 8
  • David's answer is the correct one, and should it work for you should be accepted. (and you should consider some larger buffers as he suggests, 10000 IO's is a lot for what this is doing, but good mental-floss). – WhozCraig Sep 28 '12 at 16:48
  • Thanks David, Whoz, & Ben for you all your guys' help. I went ahead and tried his code exactly has he wrote it below once it was edited and got the file to save. But I didn't need the loop because I needed to save the array once. Once I looked at the difference between his code and my original, I discovered the original problem was in my syntax. Here's the original code snippet from above as I had it before this posting. `DWORD size = (((sizeof(tile)) * tiles_total)); LPDWORD bytes_written = 0; bSuccess = WriteFile(hFile, test_buffer, size, bytes_written, NULL);` – John White Sep 28 '12 at 18:07
  • Here's what the replacement code looks like now, and sorry I can't get it to format correctly in these comment windows: `BOOL bSuccess; DWORD size = (((sizeof(tile)) * tiles_total)); DWORD bytes_written; bSuccess = WriteFile(hFile, tile_array, size, &bytes_written, NULL);` This works. Thanks again, David. – John White Sep 28 '12 at 18:08
  • Yes, I think you confused us with the 100,000 byte file write. The problem you had before you put that crazy value in was passing NULL for `lpNumberOfBytesWritten`. Glad it's sorted now. – David Heffernan Sep 28 '12 at 18:11
  • 2
    Also, when doing this for real, always adjust your bytes-i've-written counter by the returned value in dwBytesWritten. It is good practice to check that against the number of bytes you requested for write even if the function returns thumbs-up. it can be a source of some *very* subtle hard-to-find bugs. – WhozCraig Sep 28 '12 at 18:12

2 Answers2

7

Your buffer is the size of 10 ints, which is 40 bytes on Windows. You are trying to write 100,000 bytes from that buffer. That is undefined behaviour, a buffer overrun. Hence the access violation.

You must not pass a value greater than sizeof(test_buffer), i.e. 40, to the nNumberOfBytesToWrite parameter of WriteFile.

You'll need to write this file in a loop, writing 40 bytes at a time, until you have written as much as you need. Perhaps something like this:

BOOL bSuccess = TRUE;
DWORD bytesRemaining = 100000;
while (bSuccess && bytesRemaining>0)
{
    DWORD bytesToWrite = std::min(sizeof(test_buffer), bytesRemaining);
    DWORD bytesWritten;
    bSuccess = WriteFile(hFile, test_buffer, bytesToWrite, &bytesWritten, NULL);
    bytesRemaining -= bytesToWrite;
}
if (!bSuccess)
{
    //handle error;
}

Writing 40 bytes at a time is pretty slow. You'll find it more efficient to write a few KB with each call to WriteFile.

Note that you aren't allowed to pass NULL to the lpNumberOfBytesWritten parameter if you also pass NULL to lpOverlapped, as you do here. From the documentation:

lpNumberOfBytesWritten [out, optional]

......

This parameter can be NULL only when the lpOverlapped parameter is not NULL.

David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490
  • To be clear, I actually don't need to write 100,000 bytes to the file. That is merely a number I've thrown up there for testing. And it actually succeeds when the size is 100k. I had previously set the size to sizeof(test_buffer) and that's when I first started getting the access violation. Here's the code I changed it to just now: DWORD size = sizeof(test_buffer); LPDWORD bytes_written = 0; bSuccess = WriteFile(hFile, test_buffer, size, NULL, NULL); Still getting the access violation. Also tried passing in non-null DWORD for bytes_written. It told me 0 bytes were written. – John White Sep 28 '12 at 16:48
  • It's not allowed for the last two parameters to both be NULL. The `_Out_opt` annotation doesn't tell the whole story. (Found this out the hard way, since Windows 8 accepts it, but prior versions cause an access violation). – Ben Voigt Sep 28 '12 at 17:01
  • I thought I had read that about the last two parameters can't both be NULL on MSDN. I had been passing in that bytes_written LPDWORD. Whether I pass that in initialized to 0 or not initialized, I still get an access violation unless I try that ridiculously huge 100k size variable. – John White Sep 28 '12 at 17:10
  • @John your code passes NULL since NULL is equal to 0. See my updated version of the answer – David Heffernan Sep 28 '12 at 17:14
  • Got it figured out. Thanks for your help, David, as well as Ben and Whoz. I posted my eventual fix as a reply to my original post above, just in case someone else has the same problem. My formatting's all messed up though so the code is mushed together. – John White Sep 28 '12 at 18:13
  • @David: Good fix on the code, however your last sentence is incorrect. You can open a file for non-overlapped I/O, and still pass a pointer to an `OVERLAPPED` structure, in which case `lpNumberOfBytesWritten` may be NULL (the result is stored synchronously to the `OVERLAPPED` structure). – Ben Voigt Sep 28 '12 at 18:43
  • @Ben Argh, can't get it right. I think I'm there now. Thank you! – David Heffernan Sep 28 '12 at 18:48
1

You must provide a buffer to receive the number of bytes written, either the lpNumberOfBytesWritten parameter must be non-NULL, or the lpOverlapped parameter must be non-NULL.

You are passing NULL for both, which is illegal and causes the access violation.

Ben Voigt
  • 277,958
  • 43
  • 419
  • 720