reading from a binary file in C

Question

I am currently working on a project in which I have to read from a binary file and send it through sockets and I am having a hard time trying to send the whole file.

Here is what I wrote so far:

        FILE *f = fopen(line,"rt");
        //size = lseek(f, 0, SEEK_END)+1;
        fseek(f, 0L, SEEK_END);
        int size = ftell(f);
        unsigned char buffer[MSGSIZE];
        FILE *file = fopen(line,"rb");
        while(fgets(buffer,MSGSIZE,file)){
            sprintf(r.payload,"%s",buffer);
            r.len = strlen(r.payload)+1;
            res = send_message(&r);
            if (res < 0) {
                perror("[RECEIVER] Send ACK error. Exiting.\n");
                return -1;
            }   
        }

I think it has something to do with the size of the buffer that I read into,but I don't know what it's the correct formula for it.

One more thing,is the sprintf done correctly?

Might I ask why you're using `fgets`, `sprintf`, and `strlen` for a file you're intending to send as raw bytes? Shouldn't you be reading the file with `fread` directly into a `r.payload` buffer (whatever that is), updating the `r.len` to reflect the bytes read by `fread`, and sending it over the ether? — WhozCraig, Mar 21 '15 at 19:35
You should open binary files with `rb` flags, and then use `fread()` to read it. `fgets()` is designed to read lines on a text, therefore its not fit for binary content. — Havenard, Mar 21 '15 at 19:41
@Havenard Thank you. Why it doesn't state that in TFM? http://www.cplusplus.com/reference/cstdio/fgets/ — shinzou, May 29 '15 at 09:20

score 3 · Accepted Answer · edited May 23 '17 at 12:14

If you are reading binary files, a NUL character may appear anywhere in the file.

Thus, using string functions like sprintf and strlen is a bad idea. If you really need to use a second buffer (buffer), you could use memcpy. You could also directly read into r.payload (if r.payload is already allocated with sufficient size).

You are looking for fread for a binary file. The return value of fread tells you how many bytes were read into your buffer.

You may also consider to call fseek again. See here How can I get a file's size in C?

Maybe your code could look like this:

#include <stdint.h>
#include <stdio.h>

#define MSGSIZE 512

struct r_t {
  uint8_t payload[MSGSIZE];
  int len;
};

int send_message(struct r_t *t);

int main() {
  struct r_t r;
  FILE *f = fopen("test.bin","rb");
  fseek(f, 0L, SEEK_END);
  size_t size = ftell(f);
  fseek(f, 0L, SEEK_SET);
  do {
    r.len = fread(r.payload, 1, sizeof(r.payload), f);
    if (r.len > 0) {
      int res = send_message(&r);
      if (res < 0) {
        perror("[RECEIVER] Send ACK error. Exiting.\n");
        fclose(f);
        return -1;
      }
    }   
  } while (r.len > 0);
  fclose(f);
  return 0;
}

won't fred return an error in case it hits a NULL or something?how would I replace the strlen function there? — tudoricc, Mar 21 '15 at 19:40
This is from `man fread`: On success, fread() and fwrite() return the number of items read or written. — Jonas Wolf, Mar 21 '15 at 19:41

score 0 · Answer 2 · answered Mar 21 '15 at 19:35

0

No, the sprintf is not done correctly. It is prone to buffer overflow, a very serious security problem.

I would consider sending the file as e.g. 1024-byte chunks instead of as line-by-line, so I would replace the fgets call with an fread call.

Why are you opening the file twice? Apparently to get its size, but you could open it only once and jump back to the beginning of the file. And, you're not using the size you read for anything.

answered Mar 21 '15 at 19:35

juhist

4,210
16
33

Well, the answer is: you shouldn't use it at all! sprintf is an inherently unsafe function that should be used very rarely, if ever. Instead, you should read the file using fread() calls as binary blocks and send those blocks. – juhist Mar 21 '15 at 19:37
actually I think i need the size.meaning this:I have to send size chunks of that file= size * operations of send – tudoricc Mar 21 '15 at 19:37

score 0 · Answer 3 · answered Mar 21 '15 at 19:48

Is it a binary file or a text file? fgets() assumes you are reading a text file -- it stops on a line break -- but you say it's a binary file and open it with "rb" (actually, the first time you opened it with "rt", I assume that was a typo).

IMO you should never ever use sprintf. The number of characters written to the buffer depends on the parameters that are passed in, and in this case if there is no '\0' in buffer then you cannot predict how many bytes will be copied to r.payload, and there is a very good chance you will overflow that buffer.

I think sprintf() would be the first thing to fix. Use memcpy() and you can tell it exactly how many bytes to copy.

reading from a binary file in C

3 Answers3