feof becomes true prematurely

Question

I am trying to send a file over a socket by reading one character at a time until the buffer (an array of chars) is full, sending it, then repeating until the end of the file. But for some reason feof becomes true before the file ends (I believe it might be when the buffer array is full).

int c;
int end_loop = 1;
char to_send[BUFFER_LENGTH];
while (end_loop == 1) { //For the entire file
    printf("creating next data\n");
    bzero(to_send, BUFFER_LENGTH);
    for (int i = 0; i < BUFFER_LENGTH - 1; i++) { // read data from file until buffer full or EOF
        c = fgetc(f);
        if (c == EOF) { // If end of file break loop
            printf("EOF\n");
            end_loop = 0;
            break;
        }
        to_send[i] = c;
    }
    for (int i = 0; i < strlen(to_send); i++) {
        printf("%c", to_send[i]);
    }
    n = write(sockfd, to_send, strlen(to_send));
    if (n < 0) {
        perror("ERROR writing to socket");
        exit(1);
    }
}
n = write(sockfd, "EOF", 3);
if (n < 0) {
    perror("ERROR writing to socket\n");
    exit(1);
}

This is the output from a simple file

It is possible feof is not the issue, since the code seems to keep looping despite "EOF" being met.

EDIT: Added suggestions from the comments. Error still occurs.

Your check for end-of-file is flawed. You should check what [`fgetc`](http://en.cppreference.com/w/c/io/fgetc) *returns* first of all. — Some programmer dude, Aug 28 '18 at 13:03
Also note that your code both printing and sending the data doesn't care about the actual length of the input. You need to check for the end of the string (with the help of e.g. `strlen`). — Some programmer dude, Aug 28 '18 at 13:07
`c = fgetc(f); if (feof(f))` is curious code. Should `fgetc()` return `EOF` due to a rare error, loop will not exit. Suggest "c = fgetc(f); if (c == EOF)`. — chux - Reinstate Monica, Aug 28 '18 at 13:10
Code does `write(sockfd, to_send, BUFFER_LENGTH);`, yet never sets `to_send[BUFFER_LENGTH - 1]` — chux - Reinstate Monica, Aug 28 '18 at 13:15
Overall, `feof` and `ferror` are not primary-use functions. You should detect when no more data will be available from a stream by observing the return values of functions that perform I/O, such as `fgetc`. The main use of `feof` and `ferror` is to distinguish, *after the fact*, between the natural end of the string having been reached and an error having occurred on the stream. — John Bollinger, Aug 28 '18 at 13:18
@Someprogrammerdude Using `strlen()` is not a robust approach here as `fgetc(f)` may include _null characters_. — chux - Reinstate Monica, Aug 28 '18 at 13:36
Posting output text as _text_ rather than only a picture makes for a better question. Posting text only a a pic attacks down-votes. — chux - Reinstate Monica, Aug 28 '18 at 13:45
Dasonic, **specifically**, what do you see as wrong with the output? Code intentionally prints `"EOF\n"` before printing the last line. Is it that `"EOF"` that is of concern? — chux - Reinstate Monica, Aug 28 '18 at 13:52
Dasonic, on a plus side, code does attempt at some error checking - good. Note: the appended `"EOF\n"` in one place (4 characters) differs from the 3 character `write(sockfd, "EOF", 3)`. I assume that was for de-bugging, yet it is an asymmetry. — chux - Reinstate Monica, Aug 28 '18 at 14:04
@chux Thanks for all the help. It works fine now. Yes I was confused why EOF was printing early, but that my mistake in not actually paying attention to what my code did but what I thought it did. And yes the printf("EOF\n") was just for debugging, and all printing of the text is to be removed. — Dasonic, Aug 29 '18 at 01:41

chux - Reinstate Monica · Accepted Answer · 2018-08-28T13:50:26.150

Code needs to track how many characters read and then printf(), write() that many characters.

while (end_loop == 1) {
    ...
    // for (int i = 0; i < BUFFER_LENGTH - 1; i++) {
    int i;
    for (i = 0; i < BUFFER_LENGTH - 1; i++) { 
      ...
    }
    // for (int i = 0; i < BUFFER_LENGTH - 1; i++) {
    //    printf("%c", to_send[j]);
    for (int j = 0; j < i; j++) {
      ...
    // n = write(sockfd, to_send, BUFFER_LENGTH);
    n = write(sockfd, to_send, i);
    ...
}

Other problems include

Test for EOF, not only feof(f)

    c = fgetc(f);
    // if (feof(f)) { // If end of file break loop 
    if (c == EOF) { // If end of file break loop **or input error**

No need for -1 when filling a char array. A -1 might be useful with strings, but using strings here is not the best idea here either as fgetc() may return a null character.

//                                   vvv 
// for (int i = 0; i < BUFFER_LENGTH - 1; i++)
for (int i = 0; i < BUFFER_LENGTH; i++)
// or better 
size_t i; 
for (i = 0; i < sizeof to_send; i++)

OP amended code mid-answer Hmmmmmmmm.

Do not use for (int i = 0; i < strlen(to_send); i++) {. Data read may include null characters which negates good use of strings here.

If OP does not want to see "EOF\n" before the last line, print "EOF\n" after the while loop.

Sorry much of my confusion was from why EOF was printing early, then I realised that was just how I wrote it, and that the function was performing how it was meant to, and it was my server that was misbehaving. The edit was because I still thought I had an issue after the advice, when infact I did not and I am an idiot. — Dasonic, Aug 28 '18 at 13:53
@Dasonic No need for apology or self-depreciation. Instead, [pay it forward](https://en.wikipedia.org/wiki/Pay_it_forward) — chux - Reinstate Monica, Aug 28 '18 at 13:57

feof becomes true prematurely

1 Answers1