Writing and reading a binary file

Question

I am trying to to write crypt->public_key->data into a binary file. If I use size of sizeof(unsigned int) as the second argument in fwrite(), it works. However, it is declared as unsigned short * type in the header file. I am not sure why it behaves as such. Also, I face problem while writing public key and reading them as well. Although I use the size of the exactly same type in the fwrite() and fread().

Edit: It appears that the size of my crypt->public_key->data is incorrect as pointed out by usr2564301.

*I'm not sure whether I need to cite the source of this code or not. But I'll add the URL here: https://github.com/Varad0612/The-McEliece-Cryptosystem

Code from a matrix.h

typedef struct matrix
{
   int rows;             //number of rows.
   int cols;             //number of columns.
   unsigned short *data;
}*bin_matrix;

This is the code from mceliece.c

//Initialize the mceliece cryptosystem
mcc mceliece_init(int n0, int p, int w, int t)
{
    FILE *publicKey, *privateKey;
    mcc crypt;  
    crypt = (mcc)safe_malloc(sizeof(struct mceliece));  
    //crypt->code = qc_mdpc_init(n0, p, w, t);
    //crypt->public_key = generator_matrix(crypt->code);

    //printf("%d\n",crypt->public_key->rows);
    //printf("%d\n",crypt->public_key->cols);

    //Write public key into a binary file
    /*publicKey = fopen("PublicKey.bin", "wb");
    privateKey = fopen("PrivateKey.bin", "wb");

    if(privateKey != NULL){
        fwrite(crypt->code->row, n0*p*sizeof(unsigned short), n0 * p, privateKey);
        fclose(privateKey); 
    }
    else{
        printf("Unable to write private key\n");    
    }

    //Write public key into a binary file
    if(publicKey != NULL){
        fwrite(crypt->public_key->data, p*p*n0*sizeof(unsigned short), crypt->public_key->rows*crypt->public_key->cols, publicKey);
        fclose(publicKey);
    }
    else{
        printf("Unable to write public key\n");
    }*/

    //Read private key from a binary file
    crypt->code = (mdpc)safe_malloc(sizeof(struct qc_mdpc));
    crypt->code->n0 = n0;
    crypt->code->p = p;
    crypt->code->w = w;
    crypt->code->t = t;
    crypt->code->n = n0 * p;
    crypt->code->r = p;
    crypt->code->k = (n0 - 1) * p;
    crypt->code->row = (unsigned short*)calloc(n0 * p, sizeof(unsigned short));
    privateKey = fopen("PrivateKey.bin", "rb");
    if(privateKey != NULL){
        fread(crypt->code->row, p*n0*sizeof(unsigned short), p*n0, privateKey);
        fclose(privateKey); 
    }
    else
        printf("Unable to read private key\n");

    //Read public key from a binary file
    /*crypt->public_key = (bin_matrix)safe_malloc(sizeof(struct matrix));
    crypt->public_key->data = (unsigned short*)safe_malloc(p*p*n0*sizeof(unsigned short));
    crypt->public_key->rows = p;
    crypt->public_key->cols = n0*p; 
    publicKey = fopen("PublicKey.bin", "rb");
    if(publicKey != NULL){
        fread(crypt->public_key->data, p*p*n0*sizeof(unsigned short), crypt->public_key->rows*crypt->public_key->cols, publicKey);
        fclose(publicKey);  
    }
    else{
        printf("Unable to read public key\n");
    }*/

    printf("Successful\n");
    //printf("mceliece generated...\n");
    return crypt;
}

`sizeof(unsigned int)` is a constant size, supplied by your compiler. However, you *do not know* if the amount of data at `->data` is that large – you cannot derive the allocated size from a pointer. Presumably, you need to calculate its actual size from the other elements in that struct. — Jongware, Jun 24 '18 at 12:21
@SteveSummit: that gets multiplied by the earlier `sizeof(unsigned int)`. Which cannot be correct – it should probably be `sizeof(unsiged short)`. Mind, the OP is vague about what is not working. Too little data gets written? Too much? — Jongware, Jun 24 '18 at 12:30
@Lance Multiplying by `sizeof(unsigned short)` should be correct, since that's the size of the underlying data. Multiplying by `sizeof(unsigned int)` should be quite wrong, and should not work. Can you be a little more clear on how it fails when you use `unsigned short`? Are `crypt->public_key->rows` and `->cols` set properly at the point where you call `fwrite`? (Presumably the code which sets them is down in `generator_matrix()`.) — Steve Summit, Jun 24 '18 at 12:33
@SteveSummit I get a decoding failure message which is located in the decode() in qc_mdpc.c. I think that the 'crypt->public_key->rows' and '->cols' are correct as I didn't modify anything from this part in the original code. — Lance, Jun 24 '18 at 12:38

score 1 · Answer 1 · answered Jun 24 '18 at 15:12

1

There is a confusion in our reading statement:

fread(crypt->code->row, p*n0*sizeof(unsigned short), p*n0, privateKey);

This will attempt to read p*n0 elements, each with a size of p*n0*sizeof(unsigned short) bytes. If the file is not larger than the allocated size, you will be lucky as fread would not attempt to write beyond the end of the allocated block.

You should instead write:

size_t nread = fread(crypt->code->row, sizeof(unsigned short), p * n0, privateKey);
if (nread == p * n0) {
    /* private key was read successfully */
} else {
    /* file is too short, only nread words were read */
}

answered Jun 24 '18 at 15:12

chqrlie

131,814
10
121
189

`fread` does not guarantee to read as much data as requested, even if there is that much data in the file. – zwol Jun 24 '18 at 15:17
1

@zwol: actually, it does: C11 7.21.8.1p3 *The` fread` function returns the number of elements successfully read, which may be less than `nmemb` if a read error or end-of-file is encountered. If `size` or `nmemb` is zero, `fread` returns zero and the contents of the array and the state of the stream remain unchanged.* You must be referring to the unix `read` system call which may return a short count for multiple reasons even if the file has data available. – chqrlie Jun 24 '18 at 15:24
Your interpretation is plausible, but I'm not aware of _any_ implementation of `fread` that loops internally if `read` returns a short count, so as a practical matter users of `fread` must be prepared to handle short reads. – zwol Jun 24 '18 at 15:26
@zwol: really? I just looked up the glibc and found the proper loop in the internal function `_IO_default_xsgetn`. The glibc is quite a piece of work to untangle. The newlibc has the loop too in the internal function `_fread_r`. The Apple libc, derived from BSD also has the proper loop in `__fread` and `__fread0`. You must be referring to ancient or exotic implementations. If `fread` were to stop at some random location, there would be no way to restart it properly. It makes sense that it can only fail because of end-of-file or read-error, just like `getc`. – chqrlie Jun 24 '18 at 15:37
I admit to being a relic of the 1990s but I am still very surprised to learn that current `fread` implementations do contain a loop. "Never set the `size` parameter to anything other than 1" was stock advice when I learned C, precisely so you _could_ restart. – zwol Jun 24 '18 at 15:42
@zwol: We have this in common (I may even be 10 or 20 years older) and I do concede C programmers should be wary of the peculiarities of their target system. Many systems do not support C99 properly yet, It took Microsoft 16 years to make this move, but I have not seen a bogus implementation of `fread` and `fwrite` in a long time. I'm curious to be proven wrong. – chqrlie Jun 24 '18 at 15:51
@chqrlie I've changed the code as you suggested. But I don't understand why my size should be `p*n0*sizeof(unsigned short)` in fwrite() but `sizeof(unsigned short)` in fread(). If I change the size in fwrite() to `sizeof(unsigned short)`, then I'll get a decoding failure. – Lance Jun 24 '18 at 16:13
@Lance: The sizes should be consistent between `fread`, `fwrite` and `calloc`. You allocate `n0 * p * sizeof(short)` bytes with `calloc(n0 * p, sizeof(unsigned short))`, you should write the same amount of data with `fwrite(crypt->code->row, sizeof(unsigned short), p * n0, privateKey);` and read it back with `fread(crypt->code->row, sizeof(unsigned short), p * n0, privateKey);`. Your current code attempts to read **much** more data, causing undefined behavior. – chqrlie Jun 24 '18 at 16:23
@chqrlie When I do `fwrite(crypt->code->row, sizeof(unsigned short), p * n0, privateKey);`, I get a decoding error. – Lance Jun 24 '18 at 16:35
1

@Lance: do you encode and decode on the same computer? You only posted a fragment of code, I cannot tell you what is incorrect in the rest of the application. – chqrlie Jun 24 '18 at 18:58
@chqrlie I'm using the code from here: https://github.com/Varad0612/The-McEliece-Cryptosystem. I didn't change anything except trying to read and write keys in mceliece.c – Lance Jun 25 '18 at 00:48
@Lance: the package in reference does not use `fread` nor `fwrite`, it uses a text based interface to read and write the data. Your changes require a better understanding of the innerworkings of this software. – chqrlie Jun 25 '18 at 06:21

Writing and reading a binary file

1 Answers1