CODEWITHSUNDEEP
c
KawaiKx
KawaiKx

Reputation: 9940

How to prevent infinite loop writing to file

This is a c code to create a copy of a file or directory, compiled using gcc in fedora 19. It runs but it doesn't stop and I can see the new file created keeps on increasing in size ridiculously. What is wrong with this code?

#include<fcntl.h>
#include<stdio.h>
#include<unistd.h>

char buffer[2048];

int version = 1;

int main(int argc, char *argv[])
{
    void copy (int, int);

    int fdold, fdnew;

    if (argc != 3)
    {
        printf("insufficient arguments !!! \n");
        return (1);
    }

    fdold = open(argv[1], O_RDONLY);
    if (fdold == -1)
    {
        printf("cannot open file %s\n",argv[1]);
        return (1);
    }

    fdnew = creat(argv[2],0666);
    if (fdnew == -1)
    {
        printf("cannot create file %s\n",argv[2]);
        return (1);
    }

    copy (fdold, fdnew)

    return (0);
}

void copy (int fdold, int fdnew)
{
    int count;
    count = read(fdold, buffer, sizeof(buffer));

    while (count > 0)
        write(fdnew, buffer, count);
}

Upvotes: 1

Views: 674

Answers (1)

Joshua Taylor
Joshua Taylor

Reputation: 85913

You never update count and you keep writing the same data over and over again. In this code:

count = read(fdold, buffer, sizeof(buffer));
while (count > 0)
    write(fdnew, buffer, count);

You read from the file descriptor once, pulling in count bytes, and while it's greater than 0 (which is presumably is), you keep writing the buffer out to the new file. You never read any more data from the old file. If you can see the file getting bigger and bigger, you might also be able to see (depending on how the content gets flushed to disk) the same content repeated over and over again.

What you actually need to be doing is something more like this:

while there is data to read from the old file
  read it into a buffer
  while there is data in the buffer to write to the new file
    write data from the buffer into the new file

In slightly less pseudo-codish, but highly untested form, I think you'd be looking for something sort of like this:

int count = 0;
int written;
while ( 0 < (count = read(fdold, buffer, sizeof(buffer))) ) {
  written = 0;
  while ( count > (written += write(fdnew, buffer+written, count-written))) );
}

The outer loop makes sure that you read until there's nothing left to read, and the inner while make sure that you call write until written is as big as count, that is, until you've written all the bytes that you read. This is “clever” code, but it's actually too clever; you actually need to check whether written is -1, or else you'll start doing some strange things.

Something with more error checking, and hopefully more idiomatic, might be:

  for ( int count_in = -1; count_in != 0; ) {
    if ( -1 == (count_in = read(fd, buf, bufsize))) {
      perror("Problem reading from file");
      exit(-1);
    }
    else { 
      for ( int count_out = 0, out = 0; count_out < count_in; count_out += out ) {
        if ( -1 == (out = write(fd, buf+count_out, count_in-count_out)) ) {
          perror("Problem writing to file");
          exit(-1);
        }
      }
    }
  }

Upvotes: 5

Related Questions