Calculating average of all bytes in a given file in C

Question

I wrote the program but couldn't find the problem. Its reading file well but gives result 0. Did I do anything wrong on passing the file pointer?

#include <stdio.h>
unsigned char average(const char *filename){
    unsigned int BLOCK_SIZE=512;
    unsigned int nlen=0, nround=0;
    unsigned char avg = 0;
    FILE *fp;
    unsigned char tmp[512];

    if ( (fp = fopen(filename,"r")) == NULL){
            printf("\nThe file did not open\n.");
            return 500;
    }

    while(!feof(fp)){
        if(fread(tmp, 1, BLOCK_SIZE, fp)){
            nlen+=BLOCK_SIZE;
            nround++;
        }else{
            BLOCK_SIZE=BLOCK_SIZE/2;
        }
    }

    avg=(unsigned char)(nlen/nround);
    return avg;
}
int main(void){
    printf(" The average of all bytes in the file : %d \n" ,average("v") );
    return 0;
}
`

Welcome to Stack Overflow! Please see [Why is “while ( !feof (file) )” always wrong?](http://stackoverflow.com/q/5431941/2173917) — Sourav Ghosh, Jul 29 '16 at 20:09
"average of all bytes" - would you clarify the task? your code does something entirely different. — Karoly Horvath, Jul 29 '16 at 20:12
Your code roughly calculates `BLOCK_SIZE`. At the end of while loop, `nlen` should be somewhere around `nround*BLOCK_SIZE`, right? But `fread` returns the number of bytes actually read, which you are also ignoring, so it cannot possibly work if your file is smaller than `512B` (and splitting `BLOCK_SIZE` in two makes no sense). So you are filling `tmp` with 512 bytes on each call, and then doing nothing with these bytes. — vgru, Jul 29 '16 at 20:16
You can be at end of file and still feof() will return 1. feof returns zero when you try to read BEYOND the end of the file. Since you don't read the file before calling feof, you might get invalid results. — clarasoft-it, Jul 29 '16 at 20:17
Use the return value from `fread` to control the file reading loop, and to add the values read from file: it tells you the number of bytes actually read. Although you don't seem to be making any attempt to average the bytes read. And what is the point of halving the block size, when you read no bytes? Have a look at the arguemnts for `fread`, you are reading `BLOCK_SIZE` elements of 1 byte size. So the block size is 1. — Weather Vane, Jul 29 '16 at 20:29
Have you tried stepping through the program (or adding some debug printf()'s)? A few things to look at: the return value of `fread()`, when your program adjusts the value of `BLOCK_SIZE` (if ever), how `feof()` behaves at or near the end of the file (particularly for files that are exactly a multiple of 512 in size), how `fread()` behaves at or near the end of the file, what happens when a value larger than 255 is stored into `avg`. — Michael Burr, Jul 29 '16 at 20:34

score 1 · Answer 1 · edited May 23 '17 at 10:28

OP's code is roughly calculating BLOCK_SIZE count. @Groo

1) Add up the sum of all characters, one-by-one and then divide. Use a wide integer type.

unsigned long long sum = 0;
unsigned long long count = 0;
...
sum += tmp[i];
count++;

2) feof(fp) is useful after end-of-file or a read failure occurred. Do not use to to determine if another character exist. Check fread() results instead.

size_t read_count = fread(tmp, 1, BLOCK_SIZE, fp);
if (read_count == 0) Done();

3) Returning 500 make little sense for a function that returns unsigned char. Use int

4) Avoid division by 0.

5) Close the file when done.

6) Open in binary mode. @Anders K.

// avg=(unsigned char)(nlen/nround);
if (nround) avg=(unsigned char)(nlen/nround);

Put it together

#include <stdio.h>
#include <stdlib.h>

int average(const char *filename){
  FILE *fp = fopen(filename,"rb");
  if (fp == NULL){
    printf("\nThe file '%s' did not open\n.", filename);
    return -1;
  }

  unsigned long long n = 0;
  unsigned long long sum = 0;
  unsigned char tmp[512];
  size_t read_count;

  while((read_count = fread(tmp, 1, sizeof tmp, fp)) > 0) {
    n += read_count;
    for (size_t i = 0; i<read_count; i++) {
      sum += tmp[i];   
    }
  }
  fclose(fp);

  int avg = 0;
  if (n) {
    avg = (int) (sum/n);
  }
  return avg;
}

score 0 · Answer 2 · edited Aug 22 '20 at 10:43

Depending on your OS you may need to open the file with "rb" instead of "r" to avoid CR/LF translation since you use fread.

Instead of while (!foef(fp))... do something like:

size_t sz = 0;
while (sz = fread(tmp, 1, BLOCK_SIZE, fp) == BLOCK_SIZE)
{
  ...
}
nlen += sz; // to catch the last bytes

avg=(unsigned char)(nlen/nround); you don't check if nround == 0 so your program could crash by file not found or 0 bytes in file.

Calculating average of all bytes in a given file in C

2 Answers2