CODEWITHSUNDEEP
cfile-ioundefined-behavior
dean
dean

Reputation: 45

Weird behavior with C's file IO

I've been writing a virtual machine, and I've noticed some strange things have happened, even though I wrote this function ages ago. Anyhow, my virtual machine reads a file like this:

0002 000A 0001 0004 0000

However, when I don't have any whitespace after the 0000 or a new line... it crashes. Another really weird thing, which leads me to believe it's not the file loading, is that when you remove the 0000 from the file, and the whitespace... it works?! I've tried running it through GDB, but it actually works -- apparently this is called a heisenbug or something. I think it's due to the way files are loaded, which you can see in this function here on github, or just read the snippet below. 

void load_program(vm *self) {
    FILE *file = fopen("testing.ayy", "r");

    if (file != NULL) {
        if (fseek(file, 0, SEEK_END) == 0) {
            long file_size = ftell(file);
            if (file_size == -1) {
                perror("could not read filesize\n");
                exit(1);
            }
            self->source = malloc(sizeof(char) * file_size);

            if (fseek(file, 0, SEEK_SET) != 0) {
                perror("could not reset file index\n");
                exit(1);
            }

            size_t file_length = fread(self->source, sizeof(char), file_size, file);
            if (file_length == 0) {
                perror("given file is empty\n");
                exit(1);
            }
            self->source[file_size] = '\0';
        }
        fclose(file);
    }
    else {
        perror("could not read file: \n");
        exit(1);
    }

    self->source_compact = strdup(self->source);
    self->source_compact = deblank(self->source_compact);

    // here we divide because strlen is in characters,
    // whereas each instruction code should be 4 characters
    // so we're converting char size to num of instructions
    self->instructions_size = strlen(self->source_compact) / INSTRUCTION_LENGTH;

    int i;
    for (i = 0; i < self->instructions_size; i++) {
        char *instr = substring(self->source_compact, i);

        if (strcmp(instr, ERROR_CODE)) { // not equal to NOPE
            if (self->instructions != NULL) {
                self->instructions = add_instructions(self->instructions, strtol(instr, NULL, 16));
            }
            else {
                self->instructions = create_instructions(strtol(instr, NULL, 16));
            }
        }
    }

    self->instructions = reverse(self->instructions);
}

But I've added a GitHub link, since I'm not sure if it's that function; or if it's due to anything happening in the entire source -- so if any C gurus can help me, that would be brilliant :). I'm certain it's in either vm.c, or vm.h, and sorry for the terrible code; I never really looked to much into File IO when I was learning (big mistake).

Upvotes: 0

Views: 89

Answers (1)

Nisse Engstr&#246;m
Nisse Engstr&#246;m

Reputation: 4752

self->source = malloc(sizeof(char) * file_size);
...
self->source[file_size] = '\0';

You need to allocate one more byte for the terminating zero. The index source[file_size] is actually one byte beyond the end of the allocated memory. Writing into that location may clobber some other variable or the internal structures used by malloc(). Make it:

self->source = malloc(sizeof(char) * (file_size + 1));

or just:

self->source = malloc(file_size + 1);

sizeof(char) is always 1, so it is redundant.

If you want to guard yourself in case the type of self->source changes, you can use:

self->source = malloc ((file_size+1) * sizeof *self->source);

which automatically allocates the correct size.

You should also check if the allocation was successful before trying to access the memory.

Upvotes: 1

Related Questions