Why do I get an EXC_BAD_ACCESS here and how can I make it work?

Question

I'm trying to make my code be able to separate a file into a customer database (it's delimited by many spaces and not tabs). I try to use strtok, but I get an EXC_BAD_ACCESS error. Here is my main.cpp code:

#include <iostream>
#include <fstream>
#include <string>
#include <sstream>
#include "Cust.h"
using namespace std;

int main (int argc, char * const argv[]) {
    Cust customers[500];
 int idx = 0;
 string tmpString = "";
 string tmpAcctFN = "";
 string tmpAcctLN = "";
 ifstream input("P3_custData.txt");
 while (!input.eof()){
  getline(input,tmpString);
  tmpString.insert(0,"");
  customers[idx].setAcctNum(atoi(strtok((char *)tmpString.c_str()," ")));
  customers[idx].setAcctFN(strtok(NULL," "));
  customers[idx].setAcctLN(strtok(NULL," "));
  //customers[idx].setCurrBalance(atof(strtok((char *) tmpString.c_str()," ")));
 }
 cout << "return 0;";
    return 0;
}

I still get an EXC_BAD_ACCESS after making changes based on the comments:

#include <iostream>
#include <fstream>
#include <string>
#include <sstream>
#include "Cust.h"
using namespace std;

int main (int argc, char * const argv[]) {
    Cust customers[500];
    int idx = 0;
    string tmpString = "";
    string tmpAcctFN = "";
    string tmpAcctLN = "";
    char * s;
    ifstream input("P3_custData.txt");
    while (!input.eof()){
     getline(input,tmpString);
     s = strdup (tmpString.c_str());
     customers[idx].setAcctNum(atoi(strtok(s," ")));
     customers[idx].setAcctFN(strtok(NULL," "));
     customers[idx].setAcctLN(strtok(NULL," "));
     //customers[idx].setCurrBalance(atof(strtok((char *) tmpString.c_str()," ")));
    }
    cout << "return 0;";
    return 0;
}

Answer 1

It is illegal to attempt to modify the string returned by std::string::c_str() method. strtok will make such an attempt (the fact that you had to cast away the constness of the returned string is a dead giveaway). In other words, you can't use strtok on the result of std::string::c_str().

Either get rid of strtok (better), or create a standalone modifiable copy of the string and use strtok on it (worse).

Answer 2

The c_str method will return a pointer to const data. One approach is to replace:

customers[idx].setAcctNum(atoi(strtok((char *)tmpString.c_str()," ")));
customers[idx].setAcctFN(strtok(NULL," "));
customers[idx].setAcctLN(strtok(NULL," "));

with:

char *s = strdup (tmpString.c_str());
// check if s is NULL
customers[idx].setAcctNum(atoi(strtok(s," ")));
customers[idx].setAcctFN(strtok(NULL," "));
customers[idx].setAcctLN(strtok(NULL," "));
free (s);

This gives you a duplicate writeable string that you can run strtok on.

I'm not usually a big fan of using old-style C stuff in C++ code but I'm also pragmatic in getting code running. There may well be a more "C++" way of doing things.

And, if your implementation doesn't have a strdup (I don't think it's part of the standard):

char *strdup (const char *s) {
    char *d = (char *)(malloc (strlen (s) + 1));
    if (d == NULL) return NULL;
    strcpy (d,s);
    return d;
}

Update:

Anthony, based on the updates to your question, if you're getting an error still then it's probably because the string is not what you expect. Come to think of it, you should not get EXC_BAD_ACCESS from writing to the result of a c_str since the memory is technically not read-only. EXC_BAD_ACCESS should only occur if you try to write to read-only memory (or outside your address space). As one commenter has pointed out, c_str may return a pointer to read-only memory for certain simple cases but that's unlikely to be the case here if your file has complex lines.

In either case, the copying to a mutable string will fix it.

What I think may be happening is that one of your strtoks is returning NULL. This would be the case if you're processing a line with less than three fields (e.g., "hello there" where the third strtok will return NULL).

Print out the string in your code before the strtok calls:

std::cerr << "[" << s << "]" << std::endl;

(and ensure you free the string after the strtok calls lest you end up with a memory leak).

If that turns out to be the problem, the solution depends on what you need. You could ignore those lines totally by checking and copying the individual string out before calling set...() - in other words, only call them if all three strings were non-null.

You could check the string before hand to count the number of separators, ensuring that there were at least two.

I'm sure there are other possibilities but I can't really help until:

1. We know that the problem is as suspected; and
2. You tell us what you want to happen.

Hope that helps.

Answer 3

strtok() will return NULL if there are no more tokens left. You have to check for that if you hand that pointer e.g. to the constructor or assignment operator of a std::string.

char* token = ::strtok(0, " ");
if(!token) {
    // handle error
}

Still, consider using string-streams or other more C++-like functionality to avoid falling over such traps in the first place.