0

Error message is: The instruction at "0x7c810eac" referenced memory at "0x00000000". The memory could not be "written".

If I remove the destructor everything is fine. But I do not understand what is happening here. Everywhere I read that I should close handles, but here code won't let me. (Yes I know that I can do it manually... but that is a unnecessary line in client code that I think should be handled by object.)

#include <windows.h> #include <iostream> #include <string> struct fileHandle { HANDLE hFile; fileHandle(std::string path) { hFile = CreateFile(path.c_str(), GENERIC_WRITE, FILE_SHARE_READ, NULL, CREATE_NEW, FILE_ATTRIBUTE_ARCHIVE, NULL); if (hFile == INVALID_HANDLE_VALUE) { printf("error: INVALID_HANDLE_VALUE"); } } ~fileHandle() { CloseHandle(hFile); } void save(std::string data) { if (!WriteFile(hFile, data.c_str(), data.size(), NULL, NULL)) { printf("WriteFile failed: ", GetLastError()); } } }; int main() { fileHandle my_handle("test_file.txt"); my_handle.save("some text"); }

Update: this happens when file doesn't exist. When file do exist program print errors, but this is intended. I'm here asking co cover only this case when file is created (I know how to rewrite handle creating to cover existing file.)

compiler: http://sourceforge.net/projects/mingwbuilds/files/host-windows/releases/4.7.2/32-bit/threads-posix/sjlj/x32-4.7.2-release-posix-sjlj-rev7.7z

Update 2: I didn't mention that this code works and writes to file. Memory error is triggered at the very end.

Improve this question
4
  • Does this happen when the file exists or when it doesn't exist? I would have thought that as long as the file is correctly opened, it would work. So there's probably something else going on. Commented Feb 10, 2013 at 22:10
  • It happens when file doesn't exist. Commented Feb 10, 2013 at 22:10
  • There isn't any way this code can generate a null pointer exception, even if you close an invalid handle. You need to start looking for a heap corruption bug. Commented Feb 10, 2013 at 22:30
  • In a sense, Hans is correct, since it was actually the kernel that was dereferencing NULL when it tried to write back the number of bytes written. Why WriteFile() can't simply return false with a last error indicating a bad parameter, I do not know... Commented Feb 11, 2013 at 4:59

7 Answers 7

Reset to default
3

Please compile all your code with -Wall. Saves a whole lot amount of time.

In your code printf which has an invalid format string. Correct solution for printf would be (notice the %lu):

void save(std::string data) { if (!WriteFile(hFile, data.c_str(), data.size(), NULL, NULL)) { printf("WriteFile failed: %lu", GetLastError()); } }

If you would have compiled with -Wall, your code would have given the warning:

filehandle.cpp: In member function 'void fileHandle::save(std::string)': filehandle.cpp:18:50: warning: too many arguments for format [-Wformat-extra-args]

Also you should print errors to stderr as it is unbuffered. printf uses buffers and that's why you did not get any output. Also a good idea to add a \n after the errors.

Also read the other answers to improve your code. Use the rule of three.

After reading the comment I realized that this is not the segfault reason indeed. (Also look at Ron Burk's solution to see what went wrong.)

According to the Windows API documentation, lpNumberOfBytesWritten parameter can be NULL only when the lpOverlapped parameter is not NULL.

So you have to give a pointer to a DWORD where WriteFile can store how many bytes it actually read. The final save would be:

void save(std::string data) { DWORD writtenBytes; if (!WriteFile(hFile, data.c_str(), data.size(), &writtenBytes, NULL)) { printf("WriteFile failed: %lu", GetLastError()); } }

If the file exists, the error does not pop up, because passing INVALID_HANDLE_VALUE to WriteFile seems to make WriteFile return earlier than it uses your pointer.

Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

I don't think so. Too many arguments for printf() cannot lead to segfault. It just means the caller places the bogus args to stack and printf() implementation does not use them as the format string does not refer to it. The opposite case (too few args) might lead to segfault, especially if the expected arg would be pointer to be dereferenced (e.g. missing arg for format string "%s").
3

Wrong args to WriteFile(). 2nd to last argument can't be NULL. Change it to address of DWORD that you've set to 0 and all will work. It was dying in the kernel in WriteFile() as it tried to write back the # of bytes written.

Improve this answer

Comments

3

The documentation for CloseHandle makes it clear why this happens:

If the application is running under a debugger, the function will throw an exception if it receives either a handle value that is not valid or a pseudo-handle value.

So, when your call to CreateFile fails, the subsequent call to CloseHandle will raise an SEH exception.

The solution is that your code must only call CloseHandle if the call to CreateFile succeeded.

As others point out, your use of WriteFile is wrong. I won't repeat the details here.

Improve this answer

Comments

0 
~fileHandle() { if(hFile != INVALID_HANDLE_VALUE) CloseHandle(hFile); }
Improve this answer

7 Comments

@rsk82 does it make sense for fileHandle to exist if the file can't be found?
I don't think CloseHandle is supposed to cause an exception on invalid handle value, it should just return FALSE.
@LuchianGrigore: yes, because there is CREATE_NEW flag.
@rsk82 let me rephrase. Does it make sense for fileHandle to exist if it returns an invalid handle? (which also implies a new file wasn't created)
@LuchianGrigore: but I must declare this variable first, so it exist, before 'CreateFile' and after it. In first case it always is invalid.
|
0

You need to check that the file was correctly opened. 

 if (hFile != INVALID_HANDLE_VALUE) CloseFile(hFile);
Improve this answer

Comments

0

You're not following the rule of three - so ownership of hFile is shared between copies of fileHandle because of the shallow, compiler generated copy-constructor and copy-assignment operator.

For example:

fileHandle my_handle("test_file.txt"); fileHandle my_second_handle = my_handle;

Which one of these should actually close the handle on destruction? The first? What will happen when the second one goes out of scope and calls the destructor?

In this case, I'd say you should disallow copying or assignment, or pinpoint ownership.

EDIT: In your snippet, the problem is probably the one pointed out by Ron, but this is still an important point.

That's not how I'd treat it though. I think if the file couldn't be opened, fileHandle shouldn't exist. What's the point of having an invalid object? I'd simply throw an exception in the constructor to disallow invalid objects, as opposed to doing the check in the destructor.

fileHandle(std::string path) {     hFile = CreateFile(path.c_str(), GENERIC_WRITE, FILE_SHARE_READ, NULL, CREATE_NEW, FILE_ATTRIBUTE_ARCHIVE, NULL);     if (hFile == INVALID_HANDLE_VALUE) {       throw std::exception("file not found");     }   }
Improve this answer

1 Comment

Good point. In this case, I'm pretty certain disallowing copies is the right thing to do - there is "DuplicateHandle()", however in most cases, that's probably not what you want to do.
0

to be on the safe side, use

~fileHandle() { if (hfile != INVALID_HANDLE_VALUE) { CloseHandle(hfile); hfile = INVALID_HANDLE_VALUE; } }
Improve this answer

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.