CODEWITHSUNDEEP
cunused-variables
Cody
Cody

Reputation: 13

Deleting unused variable causes code to crash

So I'm trying to load the s-records from a .s19 file into memory for an assignment I'm working on, and its working. However, when I delete an unused array from my code, everything stops working and crashes.

The unused array is:

char test[65536];

And this is the loader I've written:

void loader(FILE * srec)
{
    char instring[SREC_LEN];
    char test[65536]; // This isn't used, but the program crashes without it for some reason
    int i=0;
    int j=0, k,l;
    while (fgets(instring, SREC_LEN, srec) != NULL)
    {

        while(instring[i] != '\n') // Counts the characters in the s-record
        {
            i++;

        }
        j = j+i;
        for(k=0;k<=i;k++) // Puts the records into memory
        {
            memory[l] = instring[k];
            l++;
        }
        l = j;

    }
    #ifdef DEBUG
    printf("MEMORY: %s",memory);
    #endif // DEBUG
}

If you could help me to understand why this is happening, I would appreciate it.

Upvotes: 1

Views: 374

Answers (1)

Filipe Gon&#231;alves
Filipe Gon&#231;alves

Reputation: 21223

Your code has undefined behavior, it only works by sheer luck:

fgets() may return without writing a newline character into the buffer if EOF is reached prematurely. So you should at least account for that in your loop. Also you never reset i to 0, which you should. Change this:

    while(instring[i] != '\n') // Counts the characters in the s-record
    {
        i++;

    }

to:

    i = 0;
    while(instring[i] != '\n' && instring[i] != '\0') // Counts the characters in the s-record
    {
        i++;

    }

l is never initialized; you are probably writing out of bounds in memory. Initialize l to 0:

int j = 0, k, l = 0;

(I assume that memory is large enough to hold everything).

It also looks to me like you want for(k = 0; k < i; k++) rather than for(k = 0; k <= i; k++), since i is the count of characters you want to copy.

You might want to use memcpy() instead.

Upvotes: 1

Related Questions