Access Violation With Pointers? - C++

Answer 1

An access violation usually means a bad pointer.

In this case, the most likely cause is running out of string before you find your delimiter.

Answer 2

An access violation (or "segmentation fault" on some OSes) means you've attempted to read or write to a position in memory that you never allocated.

Consider the while loop in Next():

while (*pStart != delim) // access violation error here
{
    pStart++;
}

Let's say the string is "blah\0". Note that I've included the terminating null. Now, ask yourself: how does that loop know to stop when it reaches the end of the string?

More importantly: what happens with *pStart if the loop fails to stop at the end of the string?

Answer 3

Inside ::Next you need to check for the delim character, but you also need to check for the end of the buffer, (which I'm guessing is indicated by a \0).

while (*pStart != '\0' && *pStart != delim) // access violation error here
{
    pStart++;
}

And I think that these tests in ::Next

if (pStart == NULL) { return NULL; }

Should be this instead.

if (*pStart == '\0') { return NULL; }

That is, you should be checking for a Nul character, not a null pointer. Its not clear whether you intend for these tests to detect an uninitialized pStart pointer, or the end of the buffer.

Answer 4

This answer is provided based on the edited question and various comments/observations in other answers...

First, what are the possible states for pStart when Next() is called?

1. pStart is NULL (default constructor or otherwise set to NULL)
2. *pStart is '\0' (empty string at end of string)
3. *pStart is delim (empty string at an adjacent delimiter)
4. *pStart is anything else (non-empty-string token)

At this point we only need to worry about the first option. Therefore, I would use the original "if" check here:

if (pStart == NULL) { return NULL; }

Why don't we need to worry about cases 2 or 3 yet? You probably want to treat adjacent delimiters as having an empty-string token between them, including at the start and end of the string. (If not, adjust to taste.) The while loop will handle that for us, provided you also add the '\0' check (needed regardless):

while (*pStart != delim && *pStart != '\0')

After the while loop is where you need to be careful. What are the possible states now?

1. *pStart is '\0' (token ends at end of string)
2. *pStart is delim (token ends at next delimiter)

Note that pStart itself cannot be NULL here.

You need to return pNextWord (current token) for both of these conditions so you don't drop the last token (i.e., when *pStart is '\0'). The code handles case 2 correctly but not case 1 (original code dangerously incremented pStart past '\0', the new code returned NULL). In addition, it is important to reset pStart for case 1 correctly, such that the next call to Next() returns NULL. I'll leave the exact code as an exercise to reader, since it is homework after all ;)

It's a good exercise to outline the possible states of data throughout a function in order to determine the correct action for each state, similar to formally defining base cases vs. recursive cases for recursive functions.

Finally, I noticed you have delete calls on both pStart and pNextWord in your destructor. First, to delete arrays, you need to use delete [] ptr; (i.e., array delete). Second, you wouldn't delete both pStart and pNextWord because pNextWord points into the pStart array. Third, by the end, pStart no longer points to the start of the memory, so you would need a separate member to store the original start for the delete [] call. Lastly, these arrays are allocated on the stack and not the heap (i.e., using char var[], not char* var = new char[]), and therefore they shouldn't be deleted. Therefore, you should simply use an empty destructor.

Another useful tip is to count the number of new and delete calls; there should be the same number of each. In this case, you have zero new calls, and two delete calls, indicating a serious issue. If it was the opposite, it would indicate a memory leak.