0

Ok so I'm doing a program for class when I run across a bug I've never seen before and no idea what to do about it with only minimal experience with using the debugger, so I've come here hoping someone here can set me down the path to fixing this bug. My bug is the access violation reading location. Here is the portion of code that seems to be giving me the error:

#include "Book.h" using namespace std; void add (char*, char*, int); void remove (int&); void list (); int Count; Book Bookshelf [4]; int main () { char* In = ""; char* N = ""; char* A = ""; int Y; int Num; do { cout << "Bookshelf> "; cin >> In; if (In == "add") { cout << "Bookshelf> Enter book: "; cin >> N >> A >> Y; add (N,A,Y); } else if (In == "remove") { cout << "Bookshelf> Select number: "; cin >> Num; remove (Num); } else if (In == "list") { } } while (cin != "quit"); return 0; } void add (char* N, char* A, int Y) { if (Bookshelf[4].IsEmpty() == false) cout << "Error!" << endl; else { Bookshelf[Count] = Book (N,A,Y); Count++; } cout << "Bookshelf> "; } 

I get the error when I type add into the command line to try to call the add function but it happens immediately so the debugger is no help to me. I know the problem is probably really simple but I can't seem to find it. Any help you can offer would be greatly appreciated. Let me know if any more code samples are needed.

1
  • 2
    Take a look at using std::string Commented Feb 11, 2012 at 3:32

2 Answers 2

2

You shouldn't use char* unless you really know what you are doing. For example, I rarely use char* at all and I'm programming with C++ since about 20 years. You want to use std::string, e.g. like this:

std::string In; if (std::cin >> In) { ... } 

The reason your code doesn't work is that the input operator wants to store data at the location pointed to by your pointer. However, this pointer is pointing at immutable memory for a c-string literal. When the operator tries to store something at this location it immediately gets an access violation.

The easiest fix is to use std::string. If you can't use std::string for whatever reason, use a preallocated array of char. If you do this, make sure you tell the stream how much characters are available by setting up the width:

char In[16]; if (std::cin >> std::setw(sizeof(In)) >> In) { ... } 

(the reason I'm always using a check in these example is that it is very important that you always check whether your input was successful before you do anything with the result).

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

3 Comments

Ok thank you, my new question is will my cin >> A >> Y still work to read a single line delimited by spaces if I switch to std::string?
@ildjarn Ok now my compiler is complaining that it cannot convert parameter 1 from std::string to char * on the next line with add(N,A,Y) which accepts 2 std::strings and an int. Any ideas?
@triple07 : If add takes two std::strings, then why would it try to convert to char*? Clearly there's a simple mistake in your code...
1

You cannot write data into space allocated for C-string literals: they are constants. This is the part that leads to this behavior:

char* In = ""; cin >> In; 

Two things are wrong:

  1. Your variable In does not have enough space for any non-empty string.
  2. Even if it did, writing to that space would not be allowed.

To fix this issue you could either (1) switch to using std::string (recommended) or (2) change declarations of your C strings to character arrays, like this:

char In[128]; 

5 Comments

Don't even mention MAGIC_BUFFER_SIZE.
I see, thank you, I'm going to go try switching over to std::string.
@DeadMG Why not? Recommending it is one thing, but not mentioning it at all is too much. It is a language feature whether you like it or not, and it is fully supported by the library - again, whether you like it or not. I do not like it, but I believe that programmers need to be aware of and avoid this feature consciously, as opposed to because they simply not know about it.
@dasblinkenlight: It doesn't matter why they avoid it. All that matters is that they do avoid it. Also, there is very little support for it in the C++ library. You should never mention it because some poor new guy might think it was a good idea and go use it in his code.
@DeadMG Well, I guess we have a difference of opinion on this topic then. I don't believe in dealing with the problem by hiding it: complete disclosure is usually the best medicine. As you can tell from his follow-up question, the OP used my advice, and switched to std::string.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.