downloading file from one computer to another, with socket programming

1. Create the file in binary mode as Mohibur Rashid suggested.

2. When callin fwrite write out bytesRecv number of bytes instead of sizeofbuf bytes.

3. Check the value of bytesRecv before writing the file and if its zero then the peer has shut down its send channel signaling to you basically an EOF (Note that close usually shuts down both the send and recv channels on popular OSes, many people are unaware between the difference of closing and shutting down. When a peer shuts down just its send channel you may still be able to send data to that peer if its recv channel isnt shut down. You usually get WSAECONNRESET if you try to send data to a peer that shut down its recv channel.). You don't have to handle any specific errors like WSAECONNRESET (that has nothing to do with a zero return value that is a completely different thing). Instead of handling WSAECONNRESET or any other specific error code check if bytesRecv is negative. Basically you should check only 3 cases: negative bytesRecv -> error, zero bytesRecv -> peer_shutdown_its_client_channel(eof, peer signals that it doesnt want to send more data), bytesRecv>0 -> you received bytesRecv number of bytes.

4. bytesRecv and WSAECONNRESET have nothing to do with each other, if their value are the same then its just a coincidence. When bytesRecv is negative you can print out diagnostic info for yourself if you want by querying errno on unix or WSAGetLastError() on windows, you get WSAECONNRESET and its friends here and not from bytesRecv that is negative in case of error.

5. in case of send() you are not really sending, you are just memcpy-ing data into the OS owned buffer associated with that socket. by default that buffer is only a few kilobytes. If that buffer doesnt have enough space to hold the data you want to send then only a few bytes will be copied from your buffer to the send buffer of the socket handle. In that case send() returns less than the number of bytes you wanted to write and you have to handle this. You should handle the return value of send(): zero or negative value: error, positive value: this return value indicates the number of bytes actually sent.

ifstream infile(file,ios::binary);
unsigned int filesize = Getfilesize(file.c_str()); // GetfileSize returns file size
while(!infile.eof()){
        char chunk[1000];
        int sizeofbuf = 0;
	if(filesize<1000)
	{
		sizeofbuf = filesize;
		filesize = 0;
	}
	else
	{
		sizeofbuf = 1000;
		filesize = filesize - 1000;
	}
	if(sizeofbuf==0)
		break;

	// BUG
	infile.read(chunk,sizeofbuf);

	// BUG: unhandled return value, will always work with few kilobyte files
	// that are smaller than the default send buf size of the socket, will randomly fail otherwise
	send(ParentSocket,chunk,sizeofbuf,0);

	// WHY????
	ZeroMemory(chunk,1000);
}
infile.close();

// BUG: use binary mode
FILE* fout = fopen(newname,"a+");
while(1){
	// WHY???
	ZeroMemory(Message,1000);
	int sizeofbuf=0;

	// BUG: What is this filesize related copy-paste bloat here???
	if(filesize<1000)
	{
		sizeofbuf = filesize;
		filesize = 0;
	}
	else
	{
		sizeofbuf = 1000;
		filesize = filesize - 1000;
	}
	if(sizeofbuf == 0)
		break;


	// BUG: instead of sizeofbuf use sizeof(Message)
	bytesRecv = recv( ClientSocket, Message, sizeofbuf, 0 );

	// BUG: unhandled negative return value and bytesRecv has nothing to do with error codes
	if ( bytesRecv == 0 || bytesRecv == WSAECONNRESET )

	{
		cout<<"\nConnection Closed.\n";
		
		
		return NULL;
	}

	// BUG: sizeofbuf instead of bytesRecv
	fwrite(Message,1,sizeofbuf,fout);

	// BUG: close the output only when you break the loop. i.e. in case of zero or negative return value
	fclose(fout);

	// Whats this??? You have to break the loop only in case of zero or negative bytesRecv.
	bytesRecv = 0;
	if(sizeofbuf<1000)
		break;
}
		
fclose(fout);

So an answer to the question, why doesn't this work: because this function is a big stack of bugs copy pasted from somewhere and modified afterwards.