Segmentation Fault 11 Confusion

Question

The assignment I have is reading in a test.m file, parsing it, and putting the pertinent information into an array of type statement (of which I create). I've completed the assignment for the most part. The only thing that is wrong is that I am getting a Segmentation Fault 11 at the very end of the output. Thinking it may be something wrong with the output section I'm doing, so I commented it out to see. It still gave me a seg fault. It always gives me a seg fault at the very end of the program, no matter what I comment out. I am so baffled as to what's causing it. What should I do about this? Since everything else is working, should I just ignore it?

Here is the code

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#define LC_SIZE 16
#define FREAD_SIZE 1024
#define STATES_SIZE 1024

FILE *ifp;
int linetotal;
int locals[LC_SIZE];
int localspot = 0; // localspot = # of locals
int EP;

typedef char STRING[256];

typedef struct {
  STRING label;
  STRING opcode;
  STRING arg;
} statement;

int main(int argc, char *argv[]) {

  //statement states[STATES_SIZE] = {"(NULL)"};

  char str[256];
  const char newline[3] = "\n";
  const char space[2] = " ";
  char *token;
  unsigned long length;

  ifp = (argc > 1) ? fopen(argv[1], "r") : NULL;

  if (ifp == NULL) {
    printf("Input file not available\n");
    return -1; }

  fread(str, FREAD_SIZE, 1, ifp);

  token = strtok(str, space);

  statement states[STATES_SIZE];
   for (int i = 0; i < STATES_SIZE; i++) {
    strcpy(states[i].label, "(null)");
    strcpy(states[i].opcode, "(null)");
    strcpy(states[i].arg, "(null)"); }

  while (token != NULL) {

    length = strlen(token);
    if (length > 1 ) { strcpy(str, &token[length-1]); }

    // comment check
    if ((strncmp(token, "/", 1) == 0) || (strcmp(str, "/") == 0)) {
      token = strtok(NULL, newline);
      //printf("Comment :: %s\n", token);
      token = strtok(NULL, space);
    }

    // local check
    else if (strcmp(token, "LC:") == 0) { 
      token = strtok(NULL, space);
      //printf("==LC here==\n");
      locals[localspot] = atoi(token);
      localspot++;
      //printf("Local variable %i = %s\n", localspot, token);
      token = strtok(NULL, space);
    }

    // starting point check
    else if (strcmp(token, "EP:") == 0) {
      token = strtok(NULL, space);
      EP = atoi(token);
      //printf("\nEP: %i\n", EP);
    }

    // label check
    else if (strcmp(str, ":") == 0) {
      //printf("--Label found :: %s\n", token);
      strcpy(states[linetotal].label, token);
      token = strtok(NULL, space);
    }

    // a function
    else if ((strcmp(token, "") != 0) && (strcmp(token, "\n") != 0)){
      //printf("Function :: %s\n", token);
      strcpy(states[linetotal].opcode, token);
      if ((strcmp(token, "halt") != 0) && (strcmp(token, "mul") != 0) && (strcmp(token,     "sub") != 0)) {
        token = strtok(NULL, space); 
        strcpy(states[linetotal].arg, token);
        //printf("arg :: %s\n", token);
      }
      token = strtok(NULL, space);
      linetotal++;
    }
    else
      break;
  } 

  // the output process
  printf("System State:\n-------------\n");
  printf("prog.PC: %i\n", EP);
  printf("Number of locals: %i\n", localspot);
  for (int i = 0; i < localspot; i++) 
    printf("Local %i: %i\n", i, locals[i]);
  printf("prog.PROGSIZE: %i\n\n", linetotal);

  printf("       Program      \n--------------------\n\n");
  for (int i = 0; i < linetotal; i++) 
    printf("%.6s %.5s %.6s\n", states[i].label, states[i].opcode, states[i].arg);

  fclose(ifp);
  return(0);
}

Answer 1

With fread, you read raw data from a file. That means that the string you read is not null-terminated. All the strxxx functions operate on null-terminated strings. Because str is not null-terminated, they treat the garbage after the real data as part of the string -- of undetermined length.

You should also pay attention to what WhozCraig said and make your str big enough, which also means to reserve one extra char for the terminating null character. It was a good idea to define a constant for the buffer size, but you should use it throughout your program consistently.

Anyway: null-terminate your string. fread returns the items successfully read, so:

char str[FREAD_SIZE + 1];
int n;

/* ... */

n = fread(str, 1. FREAD_SIZE, ifp);
if (n < 0) exit(1);
str[n] = '\0';

Note that I have swapped the second and third arguments to fread. The second is the size of each item, here a char. The third is the maximum number of items to read. The return value will depend on that, because it returns the number of items read, not the number of bytes read.

Edit: The char buffer str must, of course, be big enough to hold ther terminating '\0'. Otherwise setting str[n] will write beyond the buffer if the maximum number of chars are read. (Thanks for WhozCraig for spotting that. Should really have though of that myself.)