0

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); } 
2
  • 2
    dont ignore it - run gdb and do a backtrace (find out what line is causing the segfault) Commented Mar 2, 2014 at 8:16
  • 1
    fread(str, FREAD_SIZE, 1, ifp); where str is an automatic variable char str[256] in main(), and FREAD_SIZE is #define FREAD_SIZE 1024, is recipe for disaster, and it only gets worse from there. And statement states[STATES_SIZE];, where each statement is a struct of three char[256] arrays equates to adding 768KB minimum to your local stack space of main(). If it isn't brushing up against your default limit it is certainly close. Commented Mar 2, 2014 at 8:18

1 Answer 1

2

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.)

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

2 Comments

+1 I would actually read FREAD_SIZE-1 in the fread() call. If the buffer hits full capacity str[n] = 0 will write one-char past end-of-array and invoke UB. The idea is certainly correct regardless. Or declare str as FREAD_SIZE+1 length. Either way works.
Oh, yes, of course. Quite stupid to have missed this. I'll edit.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.