Question

The copy of the file has all the contents of the original and along with that something like "\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\". Also it gives error only for txt files other file formats get copied easily. I had tried using fread and fwrite earlier as well but then it wasn't giving any such problem. Is there something that I have tried wrong. It says that the file I've opened has some invalid characters in case it is a text file.

  char strr[1];
  int e;
  bzero(strr,1);
  while((e=(fread(strr,1,1,fpp)))>0)
  {
    fwrite(strr,1,1,bck);
    i++;
  }
  if(e>0)
  {
    fclose(bck);
  }

I think the problem lies in the above code.I am adding the entire code as well. Please don't comment on the multithreading part, I know it is useless when I m using mutex and when it won't reduce any time. There is a lot of work to be done. Please could you help me with the above problem. Thanks in advance.

#include<stdio.h>
#include<stdlib.h>
#include<pthread.h>
#include<stdlib.h>
#include<unistd.h>
#include<string.h>
int counter;
static FILE * fpp;
static FILE * bck;
pthread_mutex_t lock;
int wc;
int flag;
int lent;
unsigned long fsize(char* file)
{
    FILE * f = fopen(file, "rb+");
    fseek(f, 0, SEEK_END);
    unsigned long len = (unsigned long)ftell(f);
    fclose(f);
    return len;
}
void* doSomeThing(void* arg)
{
  pthread_mutex_lock(&lock);
  char str; 
  int i=0;
  counter++;
  char buff[100];
  bzero(buff,100);
  char strr[1];
  int e;
  bzero(strr,1);
  while(((e=(fread(strr,1,1,fpp)))>0) || i<=lent)  
  {
    fwrite(strr,1,1,bck);
    i++;
   }
   if(e>0)
   {
     fclose(bck);
   }
  pthread_mutex_unlock(&lock);
  return NULL;
}

int main()

{
  int i = 0;
  int err;

  char filn[100];
  printf("\nEnter Filename:");
  scanf("%s",filn);
  unsigned long len= fsize(filn);   
  lent=(int) len;
  lent=lent/5;
  printf("%lu",len);
  char backf[100];
  printf("\nEnter Filename to copy");
  scanf("%s",backf);
  bck=fopen(backf,"wb+");
  fpp=fopen(filn,"rb+");
  pthread_t tid[6];
  pthread_t * temp;
  fseek(fpp, 0, SEEK_SET);
  if (pthread_mutex_init(&lock, NULL)!=0)
  {
      printf("\n mutex init failed\n");
      return 1;
   }
  for(i=0;i<6;i++)
  err = pthread_create(&(tid[i]), NULL, &doSomeThing, NULL);
  //if (err != 0);
  int j=0;
  for(j=0;j<6;j++)
  {
    pthread_join(tid[j], NULL);
  }
  printf("\n%d",wc-1);    
  pthread_mutex_destroy(&lock);
  //fclose(bck);
  fclose(fpp);
  return 0;
}
Was it helpful?

Solution

while (e=(fread(strr,1,1,fpp)>0) )  

has the brackets wrong. It should probably be

while ((e=fread(strr,1,1,fpp))>0)

Besides, your indentation is horrible, and reading/writing 1 byte at a time is very inefficient, and it's wrong to make the fclose dependent on the if(e>0), but neither is the cause of your problem.

Brackets

fread returns the number of records read (in your case, number of bytes, as the record size is 1). It seems like you want to keep reading as long as you get a byte. But your e=(fread..)>0 compares first, then assigns the result of the comparison to e. This is actually ok in this case, because fread can return either 1 or 0 in this specific case - and you want the loop to stop when the return value isn't 1. But generally, this is often a case of an error, because people want the return value in the variable. So your statement just doesn't look right.

Indentation

is fixed now.

read/write 1 byte at a time

is extremely slow, even with the buffering that fread/fwrite provide. Processing chunks of a decently sized buffer is much faster. How big the buffer should be depends on your hardware, but 4K or 8K is a good start to use. Of course, in your specific application, you probably want to give the thread something to do instead of just copying everything in one big chunk.

make fclose dependent on (anything)

is normally wrong unless (anything) == (fopen succeded). When you're finished with the file and don't want to use it anymore, you close it. Period.

In your case, you have a loop while (e ... > 0), then an if (e>0) The part within the if condition will never get executed, as the if will never be reached with e>0. So you don't close your file anywhere. (Which, in this specific case, is a good thing, because later threads will still access it, but generally, is a mistake).

Threads

Your original post didn't mention the pthread stuff at all.

The way you use it, your first thread will perform the copy operation; while this is happening, the other threads get stuck in the mutex. After the 1st thread releases the mutex, one of the other threads will start trying to copy the file. At this point, the file pointer of the input file is at EOF, where the 1st thread left off; the next thread will try to read more data, fail, and write the un-modified, zero-initialized strr to the backup file. This repeats until the i<lent part becomes false. Same for all other threads. This is probably where your extra \000 bytes come from. Btw. this shows how important it is to post everything. Your original post discarded the i<lent part, and said nothing about multiple threads, but these are exactly the key points that lead to your undesired result.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top