0

I am trying to implement a simple file transfer. Below here is two methods that i have been testing:

Method one: sending and receiving without splitting the file. I hard coded the file size for easier testing.

sender:

send(sock,buffer,107,NULL); //sends a file with 107 size

receiver:

char * buffer = new char[107];                      
recv(sock_CONNECTION,buffer,107,0);

std::ofstream outfile (collector,std::ofstream::binary);
outfile.write (buffer,107); 

The output is as expected, the file isn't corrupted because the .txt file that i sent contains the same content as the original.

Method two: sending and receiving by splitting the contents on receiver's side. 5 bytes each loop.

sender:

send(sock,buffer,107,NULL);   

Receiver:

char * buffer = new char[107];                      //total file buffer
char * ptr = new char[5];                           //buffer
int var = 5;                
int sizecpy = size; //orig size

while(size > var ){                                //collect bytes

    recv(sock_CONNECTION,ptr,5,0);

    strcat(buffer,ptr);                     //concatenate
    size= size-var;     //decrease
    std::cout<<"Transferring.."<<std::endl;

    }

    std::cout<<"did it reach here?"<<std::endl;
    char*last = new char[size];

    recv(sock_CONNECTION,last,2,0);    //last two bytes
    strcat(buffer,last);    
    std::ofstream outfile (collector,std::ofstream::binary);
    outfile.write (buffer,107); 

Output: The text file contains invalid characters especially at the beginning and the end.

Questions: How can i make method 2 work? The sizes are the same but they yield different results. the similarity of the original file and the new file on method 2 is about 98~99% while it's 100% on method one. What's the best method for transferring files?

6
  • Your network stack hopefully prevents you, to sent only 5 bytes over the wire at a time. Praise Nagle!! Commented Jun 16, 2015 at 20:11
  • I used 5 bytes for easier testing(since .txt files are low on sizes and the contents can be easily comparable) and to know if my method will work. If it does then i plan on increasing it to like 1024 or something. Commented Jun 16, 2015 at 20:15
  • 2
    strcat is probably killing you. What you send is not necessarily going to be NULL terminated, so strcat won't know where to end. Commented Jun 16, 2015 at 20:19
  • You're ignoring the return value of recv. It returns how much bytes were actually read - you're always assuming the full 5 bytes were read, but that isn't necessarily the case. Just make sure you use the return value of recv instead of var. Commented Jun 16, 2015 at 20:21
  • 1
    You can't use strcat() into an ininitialized buffer. It will hunt through memory looking for a null before appending. Commented Jun 16, 2015 at 20:21

3 Answers 3

3

What's the best method for transferring files?

Usually I'm not answering questions like What's the best method. But in this case it's obvious:

  1. You sent the file size and a checksum in network byte order, when starting a transfer
  2. Sent more header data (e.g filename) optionally
  3. The client reads the file size and the checksum, and decodes it to host byte order
  4. You sent the file's data in reasonably sized chunks (5 bytes isn't a reasonable size), chunks should match tcp/ip frames maximum available payload size
  5. You receive chunk by chunk at the client side until the previously sent file size is matched
  6. You calculate the checksum for the received data at the client side, and check if it matches the one that was received beforhand

Note: You don't need to combine all chunks in memory at the client side, but just append them to a file at a storage medium. Also the checksum (CRC) usually can be calculated from running through data chunks.

Sign up to request clarification or add additional context in comments.

4 Comments

What i did was.. i sent headers first containing the file name and the file size and then i plan on looping until that file size is met.
o i would also like to ask, do i need to split the files sizes as well on the client side? or it's fine if i will send them in 1 go?
I am trying to make my code flexible, i am thinking what if the file size is like 14000000 bytes.
@CarloBrew TCP takes care of that for you. It will chunk up your data into little bits. On;ly reason I can think of getting in it's way is to provide user feedback.
1

Disagree with Galik. Better not to use strcat, strncat, or anything but the intended output buffer.

TCP is knda fun. You never really know how much data you are going to get, but you will get it or an error.

This will read up to MAX bytes at a time. #define MAX to whatever you want.

std::unique_ptr<char[]> buffer (new char[size]);
int loc = 0; // where in buffer to write the next batch of data
int bytesread; //how much data was read? recv will return -1 on error                        

while(size > MAX)
{                                //collect bytes
    bytesread = recv(sock_CONNECTION,&buffer[loc],MAX,0);
    if (bytesread < 0)
    {
         //handle error.
    }
    loc += bytesread;
    size= size-bytesread;     //decrease
    std::cout<<"Transferring.."<<std::endl;
}
bytesread = recv(sock_CONNECTION,&buffer[loc],size,0);
if (bytesread < 0)
{
     //handle error
}

std::ofstream outfile (collector,std::ofstream::binary);
outfile.write (buffer.get(),size); 

Even more fun, write into the output buffer so you don't have to store the whole file. In this case MAX should be a bigger number.

std::ofstream outfile (collector,std::ofstream::binary);
char buffer[MAX];
int bytesread; //how much data was read? recv will return -1 on error                        

while(size)
{                                //collect bytes
    bytesread = recv(sock_CONNECTION,buffer,MAX>size?size:MAX,0);
    // MAX>size?size:MAX is like a compact if-else: if (MAX>size){size}else{MAX}
    if (bytesread < 0)
    {
         //handle error.
    }
    outfile.write (buffer,bytesread); 
    size -= bytesread;     //decrease
    std::cout<<"Transferring.."<<std::endl;
}

7 Comments

I get error on this line outfile.write (buffer,size); when i try to use std::unique_ptr<char[]> buffer (new char[size]);
Thanks. That out to teach me not to compile. Corrected example, added better version.
Problem with method two is that once the file is lower than the max. let me try adding. ty
@CarloBrew should be handled by MAX>size?size:MAX, but it will blow up over size==0 where it should stop.. Fixing.
@CarloBrew Fixed.; Reads MAX bytes until size is smaller than MAX, then reads size until there is nothing left.
|
1

The initial problems I see are with std::strcat. You can't use it on an uninitialized buffer. Also you are not copying a null terminated c-string. You are copying a sized buffer. Better to use std::strncat for that:

char * buffer = new char[107];                      //total file buffer
char * ptr = new char[5];                           //buffer
int var = 5;                
int sizecpy = size; //orig size

// initialize buffer
*buffer = '\0'; // add null terminator

while(size > var ){                                //collect bytes

    recv(sock_CONNECTION,ptr,5,0);

    strncat(buffer, ptr, 5); // strncat only 5 chars

    size= size-var;     //decrease
    std::cout<<"Transferring.."<<std::endl;

}

beyond that you should really as error checking so the sockets library can tell you if anything went wrong with the communication.

2 Comments

This did the trick. thanks. Just one question though, is this gonna be flexible? assume that ill increase file size.
@CarloBrew Flexible in what way? You obviously need to re-write it so you can handle variable length files. If it were me I would use std::vector<char> rather than raw allocated char arrays.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.