0

I've used strdup() in the past in the same way that I am using it here. I am passing token2 into strdup which is of type char * with a valid pointer in it, yet when I try to run the line "name = strdup(token2);" my program segfaults and I am quite unsure as to why. If anyone would be able to help me it would be greatly appreciated. I also realize that my code does not return a proper type yet, I am still working on writing all of it.

struct YelpDataBST* create_business_bst(const char* businesses_path, const char* reviews_path){

  if(fopen(businesses_path,"r") == NULL || fopen(reviews_path,"r") == NULL)
    return NULL;
  FILE* fp_bp = fopen(businesses_path, "r");
  FILE* fp_rp = fopen(reviews_path, "r");

  struct YelpDataBST* yelp = malloc(sizeof(struct YelpDataBST*));

  int ID = -1;
  int tempID;
  long int addressOffset;
  long int reviewOffset;
  char line[2000];
  char line2[2000];
  char temp[2000];
  char temp2[2000];
  char* token;
  char* token2;
  char* name;
  int len;

  BusList* busNode = NULL;
  BusList* busList = NULL;
  BusTree* busTreeNode = NULL;
  BusTree* busTree = NULL;

  ID = -1;
  tempID = 0;
  fgets(line,2000,fp_rp);
  fgets(line2,2000,fp_bp);
  fseek(fp_rp,0, SEEK_SET);
  fseek(fp_bp,0,SEEK_SET);
  int ct = 0;
  while(!feof(fp_rp)){

     len = strlen(line);
     token = strtok(line, "\t");
     //printf("line: %s\n", line);
     token2 = strtok(line2, "\t");
     tempID = atoi((char*)strdup(token));
     if(ct == 0){
       tempID = 1;
       ct++;
     }

  if((ID != tempID || (ID < 0)) && tempID != 0){
    if(tempID == 1)
      tempID = 0;
    token2 = strtok(NULL, "\t");
    //name = strdup(token2);
    reviewOffset = ftell(fp_rp);
    if(tempID != 0)
      reviewOffset -= len;
    addressOffset = ftell(fp_bp);
    ID = atoi((char*)strdup(token));
    busList = BusNode_insert(busList, addressOffset, reviewOffset); //replace with create node for tree
    token2 = strtok(NULL, "\t");
    token2 = strtok(NULL, "\t");
    token2 = strtok(NULL, "\t");
    token2 = strtok(NULL, "\t");
    token2 = strtok(NULL, "\t");
    token2 = strtok(NULL, "\n");
    fgets(line2,2000,fp_bp);
  }
  token = strtok(NULL, "\t");
  token = strtok(NULL, "\t");
  token = strtok(NULL, "\t");
  token = strtok(NULL, "\t");
  token = strtok(NULL, "\n");
  fgets(line,2000,fp_rp);

  } 

  //BusList_print(busList);

}
Tom Hickey
  • 1
  • 1
  • 2
  • 1
    This line allocates the wrong number of bytes: `struct YelpDataBST* yelp = malloc(sizeof(struct YelpDataBST*));` – M.M Nov 23 '14 at 23:55
  • 1
    `tempID = atoi((char*)strdup(token));` will leak memory. (the strdup()d pointer is used but not assigned) – wildplasser Nov 23 '14 at 23:55
  • Yes, as wildplasser says; you only need `tempID = atoi(token);` (and there's no need to typecast a char* to a char*... –  Nov 24 '14 at 00:02

1 Answers1

0

strdup(token) segfaulting is most likely explained by token being NULL. (You don't need to strdup here anyway). Change that piece of code to:

if ( token == NULL )
{
    fprintf(stderr, "Invalid data in file.\n");
    exit(EXIT_FAILURE);  // or some other error handling
}

tempID = atoi(token);

However a greater problem with the surrounding code is that you are trying to use strtok twice at once. It maintains internal state and you can only have one strtok "in progress" at any one time. The second one cancels the first one. You'll have to redesign that section of code.


Also, while(!feof(fp_rp)) is wrong, and your yelp mallocs the wrong number of bytes (although in the code posted you never actually try to store anything in that storage, so it would not cause an error just yet).

Community
  • 1
  • 1
M.M
  • 138,810
  • 21
  • 208
  • 365
  • That's the weird thing, the place where the program segfaults is in the commented out section reading "name = strdup(token2)" and I have run this program through gdb and checked to see if token2 is null at the point that it segfaults, but it's not, it contains a character pointer. This is why I don't understand why it is giving me a segfault – Tom Hickey Nov 24 '14 at 00:22
  • @user3466964 you'll need to post a [MCVE](http://stackoverflow.com/help/MCVE) to have someone debug your code. As it stands there are too many unknowns, e.g. perhaps you caused heap corruption before entering this function – M.M Nov 24 '14 at 00:44
  • I figured out why it was segfaulting and have fixed the problem, thanks for the help! Could you explain how much yelp mallocs the wrong number of bytes? I am a bit confused as to what you mean by this as I am mallocing sizeof(struct YelpDataBST*). Wouldn't that just malloc the number of bytes needed for that type of struct? – Tom Hickey Nov 25 '14 at 00:24
  • @user3466964 no, that's the size of the number of bytes needed for a pointer to that type of struct. (Don't mix up pointers with the things they point to). You could take out the `*` to fix this. – M.M Nov 25 '14 at 00:30
  • @user3466964 You need to define the struct before you can `malloc` it. (Currently your function does not even use `yelp` after that anyway, so either remove this line entirely, or make sure your struct definition is visible) – M.M Nov 25 '14 at 01:39