Sign in to follow this  
jreh

file copy

Recommended Posts

My problem is that when trying to copy from one file to another the last line keeps looping, never getting to the expected NULL value at the end of the file when using fgets. The following is the code snippet where this is happening followed by the input file and output file. input_file_pointer = fopen("infile.txt", "r"); output_file_pointer = fopen("refined.txt", "w"); // read do { fgets(line_from_input, maximum_line_length, input_file_pointer); fputs(line_from_input, output_file_pointer); }while(line_from_input != NULL); // close fclose(input_file_pointer); fclose(output_file_pointer); input text file ------------------------------------------------- dadadajtfjf awfaefea gsrhddrh gfsghrdhjtfdr sgrgrgtfjf rsgrhdhrd srghrdhd srgrhgr shgrdhrdhd rhrhdhrd dhhddhrdhd tdhtrdhr rdhhdhthd dgdgfdhdtgt h output text file ------------------------------------ dadadajtfjf awfaefea gsrhddrh gfsghrdhjtfdr sgrgrgtfjf rsgrhdhrd srghrdhd srgrhgr shgrdhrdhd rhrhdhrd dhhddhrdhd tdhtrdhr rdhhdhthd dgdgfdhdtgt hhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh

Share this post


Link to post
Share on other sites
line_from_input will never be assigned NULL. What you need to do is check the return value of fgets, this is what will return NULL at the end of the file.

Something like:

do
{
if (fgets(line_from_input, maximum_line_length, input_file_pointer) == NULL) {
break;
}
fputs(line_from_input, output_file_pointer);
}while(true);



You can factor the loop to make things a bit neater, but I'm just illustrating the point here. The pointer line_from_input will never change, only the memory it points to is changed. fgets can only return NULL, it can't change line_from_input to NULL.

Share this post


Link to post
Share on other sites
!! Don't abuse NULL like that. The return from fgets is an integer (the number of characters read), not a pointer. Check against 0 instead. Better yet, don't explicitly say what you're checking against; you just want it to be non-zero, and "if (fgets(...))" is idiomatic - "if possible to get a string from the file...".

Share this post


Link to post
Share on other sites
fgets()/fputs() are very odd functions to use for file copying. I'd always prefer fread() and fwrite() under those circumstances (because there's no reason to make your program scan for line ends).

Share this post


Link to post
Share on other sites
Quote:
Original post by Zahlman
!! Don't abuse NULL like that. The return from fgets is an integer (the number of characters read), not a pointer. Check against 0 instead. Better yet, don't explicitly say what you're checking against; you just want it to be non-zero, and "if (fgets(...))" is idiomatic - "if possible to get a string from the file...".


I call bullshit.

But, yes, there are better ways to do this.

Share this post


Link to post
Share on other sites
Windows API for the win! (non-contender if this is to be platform independant code) Note that you should include your own error handling, as I have neglected to put any in.


#include <windows.h>

BOOL result = ::CopyFile(_T("infile.txt"), _T("refined.txt"), TRUE);



Heh, that'll teach you to not to give all the information. You'll note that while this answers your question, if I am not wrong, it does not solve your real problem. Which is that you actually are processing the input file instead of just copying it. Give people the info they need, and you get good answers, as opposed to correct answers that don't help.

Share this post


Link to post
Share on other sites
A portable ANSI-C version (which even works on 16-bit systems) would look something like this:


#include <stdio.h>

int /* Returns 0 if succeded else < 0 */
copyFileBuffer(const char* const source, const char* const dest, char* const temp, const int tempSize)
{
FILE* fin;
FILE* fout;
int part;
fin = fopen(source, "rb");
if (!fin)
return -1; /* Source couldn't be opened for reading */
fout = fopen(dest, "wb");
if (!fout){
fclose(fin);
return -2; /* Dest couldn't be opened for writing */
}
for (; ; ){
part = fread(temp, tempSize, 1, fin);
if (part < 0){
fclose(fout);
fclose(fin);
return -3; /* Couldn't read from source file */
}
if (part == 0)
break;
if (fwrite(temp, part, 1, fout) != part){
fclose(fout);
fclose(fin);
return -4; /* Couldn't write to dest file */
}
if (part < tempSize)
break;
}
fclose(fout);
fclose(fin);
return 0;
}

/* Example, copy using 16kb of stack as temporary */

int /* Returns 0 if succeded else < 0 */
copyFileStack(const char* const source, const char* const dest)
{
char temp[16384];
return copyFileBuffer(source, dest, &temp[0], 16384);
}

/* Example, copy using 16kb of heap as temporary */

static char tempHeap[16384];

int /* Returns 0 if succeded else < 0 */
copyFileHeap(const char* const source, const char* const dest)
{
return copyFileBuffer(source, dest, &tempHeap[0], 16384);
}





Disclaimer: I haven't actually tested the code above.

Edit: Made it easier to reuse code for different approaches.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this