fread/fwrite 2

B

Bill Cunningham

Ok after looking a Nick's old post again, I decided to try my own
copy. It seems to fill better with my style. Now it compiles can
copies fine. The thing is it hasn't been tested as to whether or not
it copied actual 2048 bytes at a time. I will submit it for review...

#include <stdio.h>

int main()
{
size_t nread = 0;
char buf[2048];
FILE *fpi, *fpo;
if ((fpi = fopen("t", "rb")) == NULL)
return -1;
if ((fpo = fopen("z", "wb")) == NULL)
return -2;
do {
nread = fread(buf, sizeof(buf), 1, fpi);
if (ferror(fpi))
return -3;
fwrite(buf, sizeof(buf), nread, fpo);
if (ferror(fpo))
return -4;
}
while (!feof(fpi));
fclose(fpi);
fclose(fpo);
return 0;
}
 
B

Bill Cunningham

Ok after looking a Nick's old post again, I decided to try my own
copy. It seems to fill better with my style. Now it compiles can
copies fine. The thing is it hasn't been tested as to whether or not
it copied actual 2048 bytes at a time. I will submit it for review...

#include <stdio.h>

int main()
{
    size_t nread = 0;
    char buf[2048];
    FILE *fpi, *fpo;
    if ((fpi = fopen("t", "rb")) == NULL)
        return -1;
    if ((fpo = fopen("z", "wb")) == NULL)
        return -2;
    do {
        nread = fread(buf, sizeof(buf), 1, fpi);
        if (ferror(fpi))
            return -3;
        fwrite(buf, sizeof(buf), nread, fpo);
        if (ferror(fpo))
            return -4;
    }
    while (!feof(fpi));
    fclose(fpi);
    fclose(fpo);
    return 0;



}- Hide quoted text -

- Show quoted text -

Ok I guess there must be a short count somewhere in this code. Most of
it has been copied I get no errors and I must admit I'm a little
stumped. It's got to be in the fread fwrite system somehow.

B
 
B

Bill Cunningham

Ok after looking a Nick's old post again, I decided to try my own
copy. It seems to fill better with my style. Now it compiles can
copies fine. The thing is it hasn't been tested as to whether or not
it copied actual 2048 bytes at a time. I will submit it for review...

#include <stdio.h>

int main()
{
    size_t nread = 0;
    char buf[2048];
    FILE *fpi, *fpo;
    if ((fpi = fopen("t", "rb")) == NULL)
        return -1;
    if ((fpo = fopen("z", "wb")) == NULL)
        return -2;
    do {
        nread = fread(buf, sizeof(buf), 1, fpi);
        if (ferror(fpi))
            return -3;
        fwrite(buf, sizeof(buf), nread, fpo);
        if (ferror(fpo))
            return -4;
    }
    while (!feof(fpi));
    fclose(fpi);
    fclose(fpo);
    return 0;



}- Hide quoted text -

- Show quoted text -

Ok my apoligies all. It looks like in the time it took me to look at
Nick K's old code and write my own I ended up getting a couple of
parameters backwards. Looks like a write error to me. Right? If
anyones has any input I'd appreciate.

B
 
I

Ike Naar

int main(void)
{
? ? size_t nread = 0;
? ? char buf[2048];
? ? FILE *fpi, *fpo;
? ? if ((fpi = fopen("t", "rb")) == NULL)
? ? ? ? return -1;
? ? if ((fpo = fopen("z", "wb")) == NULL)
? ? ? ? return -2;
? ? do {
? ? ? ? nread = fread(buf, sizeof(buf), 1, fpi);

The second parameter of fread specifies the size of the
elemtents to be read, and the third parameter specifies the
(maximal) number of them. If you're going to read bytes instead
of 2048-byte blocks, it would be more natural to use

nread = fread(buf, 1, sizeof buf, fpi);

and, to match the change in the fread arguments, here it would be

fwrite(buf, 1, nread, fpo);
 
J

John Gordon

In said:
Ok my apoligies all. It looks like in the time it took me to look at
Nick K's old code and write my own I ended up getting a couple of
parameters backwards. Looks like a write error to me. Right? If
anyones has any input I'd appreciate.

If the program isn't behaving the way you want, tell us that. Say
exactly what it is doing wrong.

Does it not compile?
Does it run, but print an error message?
Does it run, but not produce the desired output?

Vague comments like "Looks like a write error" don't help us track down
the problem.
 
B

Bill Cunningham

If the program isn't behaving the way you want, tell us that.  Say
exactly what it is doing wrong.

Does it not compile?
Does it run, but print an error message?
Does it run, but not produce the desired output?

Vague comments like "Looks like a write error" don't help us track down
the problem.

Well It's like I said above it's a short count. diff says the files
differ. I'm not wanting to copy 2048 bytes but 2048 byte sized blocks.
I can copy bytes with fputs and fputc. I want a copy of the entire
file.

B
 
B

Ben Bacarisse

Bill Cunningham said:
Well It's like I said above it's a short count. diff says the files
differ. I'm not wanting to copy 2048 bytes but 2048 byte sized blocks.
I can copy bytes with fputs and fputc. I want a copy of the entire
file.

Did you miss the correct analysis of the bug? It was posted yesterday by
Ike Naar.

Another strategy would have been to compare all the changes you made
from the original working code. That would have enabled you to find the
bug that Ike pointed out by yourself.
 
B

Bill Cunningham

Did you miss the correct analysis of the bug?  It was posted yesterday by
Ike Naar.

Both the posts were written by me at the same time. 6:18 and 6:32 PM
were when each were posted. Then Ike responded at 6:42. Why it took so
long for the 2nd post to post I don't know.
Another strategy would have been to compare all the changes you made
from the original working code.  That would have enabled you to find the
bug that Ike pointed out by yourself.
I did. My original post was premature.

Bill
 
B

Bill Cunningham

int main(void)
{
? ? size_t nread = 0;
? ? char buf[2048];
? ? FILE *fpi, *fpo;
? ? if ((fpi = fopen("t", "rb")) == NULL)
? ? ? ? return -1;
? ? if ((fpo = fopen("z", "wb")) == NULL)
? ? ? ? return -2;
? ? do {
? ? ? ? nread = fread(buf, sizeof(buf), 1, fpi);

The second parameter of fread specifies the size of the
elemtents to be read, and the third parameter specifies the
(maximal) number of them. If you're going to read bytes instead
of 2048-byte blocks, it would be more natural to use

        nread = fread(buf, 1, sizeof buf, fpi);

and, to match the change in the fread arguments, here it would be

        fwrite(buf, 1, nread, fpo);




Ok I guess there must be a short count somewhere in this code. Most of
it has been copied I get no errors and I must admit I'm a little
stumped. It's got to be in the fread fwrite system somehow.

Thanks Ike and I hope this post was not premature. I altered my code
some taking your advice and looking at Nick's old code. Here's what I
have now and it compiles and runs fine and a complete file is copied.
I think in 2048 blocks too. Here's a copy of my amended code...

#include <stdio.h>

int main()
{
size_t nread = 2048;
char buf[2048];
FILE *fpi, *fpo;
if ((fpi = fopen("t", "rb")) == NULL)
return -1;
if ((fpo = fopen("z", "wb")) == NULL)
return -2;
do {
nread = fread(buf, 1, sizeof buf, fpi);
if (ferror(fpi))
printf("error 3");
fwrite(buf, 1, nread, fpo);
if (ferror(fpo))
printf("error 4");
}
while (!feof(fpi));
fclose(fpi);
fclose(fpo);
return 0;
}

Bill
 
B

Ben Bacarisse

Bill Cunningham said:
Thanks Ike and I hope this post was not premature. I altered my code
some taking your advice and looking at Nick's old code. Here's what I
have now and it compiles and runs fine and a complete file is copied.
I think in 2048 blocks too. Here's a copy of my amended code...

#include <stdio.h>

int main()

Why not int main(void)? It may not matter to you here, but it's a good
idea to get into the habit of using the best C idioms everywhere.
{
size_t nread = 2048;

I prefer not to initialise variables unless the value is actually
useful.
char buf[2048];
FILE *fpi, *fpo;
if ((fpi = fopen("t", "rb")) == NULL)
return -1;
if ((fpo = fopen("z", "wb")) == NULL)
return -2;

Are negative return values useful to you on your system? They aren't
very useful on mine.

A program that has hard-wired file names like this is not going to be
useful. I know this is learning exercise, but the next step would be to
learn how to get rid of this restriction.
do {
nread = fread(buf, 1, sizeof buf, fpi);
if (ferror(fpi))
printf("error 3");
fwrite(buf, 1, nread, fpo);
if (ferror(fpo))
printf("error 4");

The error handling is inconsistent. You print a message for two errors
and use non-zero return values for the others. It suggest you haven't
really decided what to do with errors. Also, errors should be printed
to stderr -- that's what it's for after all.
}
while (!feof(fpi));

There's a bug here. What happens when there is a read error?
fclose(fpi);
fclose(fpo);

There's a another inconsistency here. You close the streams when all
goes well, but not when the output file can't be opened. It's a good
idea to get into the habit of closing all open streams.
return 0;
}

<snip>
 
B

Bill Cunningham

Why not int main(void)?  It may not matter to you here, but it's a good
idea to get into the habit of using the best C idioms everywhere.

'cause no one usually uses it. () is like a synonym if I'm right.
I prefer not to initialise variables unless the value is actually
useful.

Why is it not useful in the fwrite 3rd parameter?
    char buf[2048];
    FILE *fpi, *fpo;
    if ((fpi = fopen("t", "rb")) == NULL)
   return -1;
    if ((fpo = fopen("z", "wb")) == NULL)
   return -2;

Are negative return values useful to you on your system?  They aren't
very useful on mine.

All negative numbers are errors. Of course I'd have to call main()
manually get these useful. So I changed the code to print error 3 and
error 4. But didn't completely re-write the whole code.
A program that has hard-wired file names like this is not going to be
useful.  I know this is learning exercise, but the next step would be to
learn how to get rid of this restriction.

I decided not to use the infamous main(int argc,char *argv[])
The error handling is inconsistent.  You print a message for two errors
and use non-zero return values for the others.  It suggest you haven't
really decided what to do with errors.  Also, errors should be printed
to stderr -- that's what it's for after all.

Normally in code I direct to stderr. I just used printf for brevity.
There's a bug here.  What happens when there is a read error?

What's exactly wrong here? Should I maybe have used
while(ferror(fpi)&&!feof(fpi)); ?

I should've written a small function like int Fclose(FILE *f1,FILE
*f2){
fclose(f1);
fclose(f2);
return 0;
}

So I wouldn't have to type fclose so much.
 
B

Ben Bacarisse

Bill Cunningham said:
'cause no one usually uses it. () is like a synonym if I'm right.

It's not a synonym, but it's very similar. In C, adding void is better
-- you usually get more help from the compiler if you write an erroneous
call to the function.
Why is it not useful in the fwrite 3rd parameter?

because it gets assigned before then.

What's exactly wrong here? Should I maybe have used
while(ferror(fpi)&&!feof(fpi)); ?

Not quite. You need to stop when there is an error (at least when there
is a read error) because you can't rely on eof(fpi) becomming true.
I should've written a small function like int Fclose(FILE *f1,FILE
*f2){
fclose(f1);
fclose(f2);
return 0;
}

So I wouldn't have to type fclose so much.

That's not a very string reason to write a function.

Is the idea of a file closing function related to this?
 
B

Bill Cunningham

Not quite.  You need to stop when there is an error (at least when there
is a read error) because you can't rely on eof(fpi) becomming true.

What could I do then? Test for ferror and do something else with
feof ?
 
K

Keith Thompson

Bill Cunningham said:
What could I do then? Test for ferror and do something else with
feof ?

For the benefit of other readers:

First check the value returned by fread() or fwrite(). If that result
indicates an error, *then* call feof() and/or ferror() to get more
information.

[...]
 
B

Bill Cunningham

[snip]
The second parameter of fread specifies the size of the
elemtents to be read, and the third parameter specifies the
(maximal) number of them. If you're going to read bytes instead
of 2048-byte blocks, it would be more natural to use

        nread = fread(buf, 1, sizeof buf, fpi);

[snip]

Ok and if I wanted to read 2048 bytes blocks then would I want this
instead?

nread=fread(buf,sizeof buf,1,fpi);

makes sense. Remember I'm pretty unfamilliar with these functions. I
usually use fgetc

Bill
 
B

Ben Bacarisse

Bill Cunningham said:
[snip]
The second parameter of fread specifies the size of the
elemtents to be read, and the third parameter specifies the
(maximal) number of them. If you're going to read bytes instead
of 2048-byte blocks, it would be more natural to use

        nread = fread(buf, 1, sizeof buf, fpi);

[snip]

Ok and if I wanted to read 2048 bytes blocks then would I want this
instead?

nread=fread(buf,sizeof buf,1,fpi);

makes sense. Remember I'm pretty unfamilliar with these functions. I
usually use fgetc

Ike's post has led you astray (not deliberately, I'm sure). Both forms
read 2048-byte blocks when they can. The difference comes when you get
close to the end of the file. When there are fewer than 2048 bytes left
the first form

nread = fread(buf, 1, sizeof buf, fpi);

reads what it can and puts the number of bytes read in nread. You need
that because you want to write exactly that number of bytes in the
subsequent call to fwrite. If you use this:

nread = fread(buf, sizeof buf, 1, fpi);

nread will be 0 since zero 2048-byte objects were read. Now you don't
know how much, if any, of the bytes in buf need to be written out.
 
G

Guest

For the benefit of other readers:

First check the value returned by fread() or fwrite(). If that result
indicates an error, *then* call feof() and/or ferror() to get more
information.


which is what the original code did...
 
B

Ben Bacarisse

which is what the original code did...

Yes. In fact, if anyone is using this thread to learn up on C's IO they
should compare your original posting with Bill's latest re-write.
As far as I can tell, every change to the code has made it slightly
worse, so all the differences are significant.

It's a good way to learn. I used to do something similar when teaching.
Rather than just post a single model answer, I'd post some of the best
student answers as well. That way it was likely that there was at least
one good example that matched up with every reasonable approach to a
problem.

In that spirit, here is my "programming 101" submission. It uses quite
a different structure and style:

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

int main(int argc, char *argv[])
{
FILE *fpi = argc > 1 ? fopen(argv[1], "rb") : NULL;
FILE *fpo = argc > 2 ? fopen(argv[2], "wb") : NULL;
if (fpi && fpo) {
size_t nread;
char buf[2048];
while ((nread = fread(buf, 1, sizeof buf, fpi)) > 0 &&
fwrite(buf, 1, nread, fpo) == nread)
/* do nothing */;

int copied_ok = feof(fpi) && !ferror(fpo);
if (fclose(fpi) + fclose(fpo) == 0 && copied_ok)
return EXIT_SUCCESS;
fprintf(stderr, "Error copying file.\n");
}
else if (fpi) {
fclose(fpi);
fprintf(stderr, "Unable to open output file.\n");
}
else if (fpo) {
fclose(fpo);
fprintf(stderr, "Unable to open input file.\n");
}
return EXIT_FAILURE;
}
 
B

Bill Cunningham

Bill Cunningham said:
The second parameter of fread specifies the size of the
elemtents to be read, and the third parameter specifies the
(maximal) number of them. If you're going to read bytes instead
of 2048-byte blocks, it would be more natural to use
        nread = fread(buf, 1, sizeof buf, fpi);

Ok and if I wanted to read 2048 bytes blocks then would I want this
instead?
nread=fread(buf,sizeof buf,1,fpi);
makes sense. Remember I'm pretty unfamilliar with these functions. I
usually use fgetc

Ike's post has led you astray (not deliberately, I'm sure).  Both forms
read 2048-byte blocks when they can.  The difference comes when you get
close to the end of the file.  When there are fewer than 2048 bytes left
the first form

  nread = fread(buf, 1, sizeof buf, fpi);

reads what it can and puts the number of bytes read in nread.  You need
that because you want to write exactly that number of bytes in the
subsequent call to fwrite.  If you use this:

  nread = fread(buf, sizeof buf, 1, fpi);

nread will be 0 since zero 2048-byte objects were read.  Now you don't
know how much, if any, of the bytes in buf need to be written out.

Ok. I am slowly coming around to this. Would it be a good idea while
trying to grasp this to forget ferror and feof? Are they really
needed? I have copied your latest code. It's alittle beyond my reading
abillity since there is so much shorthand. And your rewrite of Nick's
original code my compiler said hand a couple of errors. ) expected
before { or something trivial like that. I just in my code didn't use
a command line interface with main(int argc,char **argv).

Bill
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Similar Threads

Can not read VCD file in Linux 8
error 28
file bug 28
fread fwrite struct 4
A Question of Style 51
Bug with compiler or am I just doing something illegal 26
fwrite() fails when called after fread() 2
fwrite 10

Staff online

Members online

Forum statistics

Threads
473,731
Messages
2,569,432
Members
44,832
Latest member
GlennSmall

Latest Threads

Top