Writing binary to a file is pulling incorrect data from previous unrelated popen commands in C

Question

So this code functions by itself, but when I use it with my main program, it's somehow pulling in some, what seems to be, completely unrelated parts of the code and writing it to the file I'm writing... All compiles without issues, so I know it's not a definition problem or a pointer issue, and everything else writes properly, but somehow I'm getting over 3200 bytes of stuff that the struct doesn't have any relation to, and writing it at address 0x1 in the file, which isn't any part of the struct either...

struct a {
    unsigned long addr; //File address
    int sz; //Num Bytes
    unsigned long pos; // Buffer Address
};

// Many more than this, but you get the general struct idea..
struct a as[][3] = {
    {{ 0xF245, 5, 0x6F02C4 }},
    {{ 0x471D, 128, 0x65892 }},
    {{ 0x6198F, 12, 0xA4092 }}
}

//Failing code
        fdin = fopen(files[FIRSTFILE]->filename, "rb");

        fdout = fopen(files[SECONDFILE]->filename, "r+b");

        if (!fdin) {
            fprintf(stderr, "Unable to open %s\n", files[FIRSTFILE]->filename);
            fclose(fdin);
            cleanup(ONSCREEN);
            return EXIT_FAILURE;
        }

        if (!fdout) {
            fprintf(stderr, "Unable to open %s\n", files[SECONDFILE]->filename);
            fclose(fdout);
            fclose(fdin);
            cleanup(ONSCREEN);
            return EXIT_FAILURE;
        }

I have other code here, but none that read from a file and write to another like this, But somewhere in here it's writing at least 3200 bytes incorrectly in the range address 0x1-0xC88 in the file and pulling in data that I'm using in popen functions before all of this.

        for (int i = 0; i <= (sizeof(buffer) / sizeof(buffer[0])); i++) {

            memset(buffer, 0, sizeof(buffer));
            fseek(fdin, as[i]->pos, SEEK_SET);
            fread(buffer, 1, as[i]->sz, fdin);

            fseek(fdout, as[i]->addr, SEEK_SET);
            fwrite(buffer, 1, as[i]->sz, fdout);
        }

        if(fclose(fdout)==EOF || fclose(fdin)==EOF) {
            logit(ONSCREEN, "Error closing files.\n\n");
            cleanup(ONSCREEN);
            return EXIT_FAILURE;
        }
        fflush(fdin);
        fflush(fdout);

Here's a piece of the code from the main program that somehow it's pulling information from:

        sleep(1);
        memset(command, 0x00, 256);
        sprintf(command, "./somecommand");
        fp = popen(command, "r");
        if (fp == NULL) {
            logit(ONSCREEN, "popen failed.");
            cleanup(ONSCREEN);
            return EXIT_FAILURE;
        }

        while(fgets(store, sizeof(store), fp)) {
            if (strstr(store, "Expected Output")) {
            break;
            }
        }
        pclose(fp);
        fflush(fp);

Again, all of these function just fine by themselves, but when put together in a single function, they don't play well together... The files (FILE *fp, *fdin, *fdout) are differently named, and the store character array is named differently than buffer. What have I done wrong here?

Seems to be something unsafe about using popen and fopen in the same function like that or something I'm not clearing out properly here...?

Answer 1

In your example, as[] has 3 elements (your real-life code may have a different number)

// Many more than this, but you get the general struct idea..
struct a as[][3] = {
    {{ 0xF245, 5, 0x6F02C4 }},
    {{ 0x471D, 128, 0x65892 }},
    {{ 0x6198F, 12, 0xA4092 }}
}

However, you're using the number of elements in buffer (which in a comment you say is char buffer[256] to index it:

   for (int i = 0; i <= (sizeof(buffer) / sizeof(buffer[0])); i++) {

        memset(buffer, 0, sizeof(buffer));
        fseek(fdin, as[i]->pos, SEEK_SET);
        fread(buffer, 1, as[i]->sz, fdin);

        fseek(fdout, as[i]->addr, SEEK_SET);
        fwrite(buffer, 1, as[i]->sz, fdout);
    }

Change the for loop to (note also that the test is changed from <= to <):

  for (int i = 0; i < (sizeof(as) / sizeof(as[0])); i++)  

Finally - I think you're making things unnecessarily more complicated (and probably buggy) by using a 2 dimensional array for as for no reason. try:

struct a as[] = {
    { 0xF245, 5, 0x6F02C4 },
    { 0x471D, 128, 0x65892 },
    { 0x6198F, 12, 0xA4092 }
}


// ...
   for (int i = 0; i < (sizeof(as) / sizeof(as[0])); i++) {

        memset(buffer, 0, sizeof(buffer));
        fseek(fdin, as[i].pos, SEEK_SET);
        fread(buffer, 1, as[i].sz, fdin);

        fseek(fdout, as[i].addr, SEEK_SET);
        fwrite(buffer, 1, as[i].sz, fdout);
    }