alloc storage and binary fwrite shorts in c

Question

I am trying to allocate a single block of shorts, fwrite it to a file, and then read it back. But the data that gets written into the file doesn't match what is coming out. I've isolated the problem to the following bit of code. Any ideas what I'm doing wrong?

#define CHUNK_SIZE 1000
void xwriteStructuresToFile(FILE *file, void * structureData)
{
    assert((fwrite(structureData, sizeof(short), CHUNK_SIZE, file)) == CHUNK_SIZE);

}

void wwbuildPtxFiles(void)
{   
    FILE *file = fopen("s:\\tv\\run32\\junky.bin", WRITE_BINARY);
    int count = 10;
    short *ptx = (short *) calloc(CHUNK_SIZE * count, sizeof(short ) );

    memset(ptx, '3', sizeof(short) * CHUNK_SIZE * count);
    for (int dayIndex = 0; dayIndex < count; ++dayIndex)
        xwriteStructuresToFile(file, (void *) &ptx[ CHUNK_SIZE * sizeof(short) * dayIndex ]);

    free(ptx);
    fclose(file);

    file = fopen("s:\\tv\\run32\\junky.bin", READ_BINARY);
    int xcount = CHUNK_SIZE * count * sizeof(short );
    for (int i = 0; i < xcount; ++i)
    {
        char x;
        if ((x = getc(file)) != '3')
            assert(false);
    }
}

Don't do `assert()` on operational code...It is crucial that the `fwrite()` occurs, even if compiled with `-DNDEBUG`. — Jonathan Leffler, Feb 14 '12 at 05:32

score 2 · Answer 1 · answered Feb 14 '12 at 05:35

2

Remove sizeof(short) from the array index. C will do this calculation for you

answered Feb 14 '12 at 05:35

leif

1,987
4
19
22

score 1 · Answer 2 · answered Feb 14 '12 at 05:33

In your call to xwriteStructuresToFile, you use:

&ptx[ CHUNK_SIZE * sizeof(short) * dayIndex ]

ptx is a short pointer, meaning array calculations will automatically be scaled up to the size of a short.

By explicitly doing this as well in the above expression, you're moving well beyond the end of the array. You need to replace that line with something like:

xwriteStructuresToFile(file, &ptx[CHUNK_SIZE * dayIndex]);

score 1 · Answer 3 · answered Feb 14 '12 at 05:39

You're writing the 'data' beyond the end of the array!

xwriteStructuresToFile(file, (void *) &ptx[ CHUNK_SIZE * sizeof(short) * dayIndex ]);

You should be using:

xwriteStructuresToFile(file, &ptx[CHUNK_SIZE * dayIndex]);

The C compiler scales by sizeof(short) automatically. If you have an array of integers, you don't write array[i * sizeof(int)] to access the i^th member of the array; similarly, here you don't need to scale the index by sizeof(short). Indeed, it is crucial that you don't because you are writing twice (on the assumption that sizeof(short) == 2) as far through the memory as you expected.

You also should not use assert() around a function call that must be executed. You use assert() in a separate statement that can be omitted from the program without affecting its function. This is discussed at some length in 'Writing Solid Code' by Steve Maguire, which is a bit dated in places, but sound on this point, at least.

AndersK · Accepted Answer · 2012-02-14T06:20:48.570

A few things:

the way you open the file, I am not sure about your constants but they should read

"wb" to write a binary file and "rb" to read.

Never put a statement in an assert, an assert is removed when the program is compiled in release mode. Instead, check return value and assert on that

e.g.

bool ok =fwrite(structureData, sizeof(short), CHUNK_SIZE, file)) == CHUNK_SIZE;
assert(ok);

although you should not assert on this, instead you should print out a proper error message. assert are for programming errors, not runtime errors.

short *ptx = (short *) calloc(CHUNK_SIZE * count, sizeof(short ) );

the above line contains a number of issues:

never cast return value of calloc in C. short *ptx = calloc... should suffice, if you get a warning, #include <stdlib.h>
you should use the form calloc( count, CHUNK_SIZE * sizeof( short )); it otherwise looks a bit unclear. ( calloc takes number,size as arguments)

   for (int dayIndex = 0; dayIndex < count; ++dayIndex)
      xwriteStructuresToFile(file, 
         (void *) &ptx[ CHUNK_SIZE * sizeof(short) * dayIndex ]);

not sure what you are doing there, replace the two statements with

fwrite( ptx, CHUNK_SIZE * sizeof( short ), count, fp );

that should write the whole array.

score 0 · Answer 5 · answered Feb 14 '12 at 05:34

0

Because ptx is a short * pointer, you shouldn't multiply by sizeof(short) when you index it. The index is in units of shorts already, so you want:

xwriteStructuresToFile(file, (void *) &ptx[ CHUNK_SIZE * dayIndex ]);

answered Feb 14 '12 at 05:34

caf

233,326
40
323
462

alloc storage and binary fwrite shorts in c

5 Answers5