mark_c wrote:How come the two versions of the same code below produce different results on the file?
For example, if I read binary files from sockets, I have differences.
You clearly have not been paying attention, because your code examples still have several errors that I have already addressed in earlier replies. Please go back and re-read the history of this discussion thread again more carefully:
- You never said which version of C++Builder you are using. If CB2009+, String is UnicodeString, not AnsiString. Notice in my examples that I changed String to AnsiString. This is important. Otherwise, you should use a more suitable byte container instead, such as std::vector<Byte>.
- You are ignoring ByteRecived when appending Buf to MyBuffer. You are assuming Buf is null-terminated, but it is not. You must take ByteRecived into account when appending Buf to MyBuffer.
- You are ignoring Size while looping, so you loop more than you should be.
As for why your two examples are producing different file outputs - it is because they both have errors in their respective logics.
In the first example, you are potentially writing garbage bytes to MyBuffer due to mishandling of null terminators. But more importantly, the 'for' loop that saves MyBuffer to file is skipping the last byte saved in MyBuffer, as it is using '<' instead of '<='. Since it is looping through a 1-indexed string, '<' will ignore the last index.
In the second example, the 'for' loop is skipping the first byte received in Buf, because it is starting at index 1 instead of 0. Buf is a 0-indexed array, not a 1-indexed string.
In both cases, it would be a lot easier to just call fwrite() one time with the entire data instead of calling fprintf() in a loop:
Code: Select all
void __fastcall TForm1::ClientSocket1Read(TObject *Sender, TCustomWinSocket *Socket)
{
AnsiString MyBuffer;
int ByteRecived;
int Size = Socket->ReceiveLength();
if (Size <= 0) return;
char *Buf = new char[Size];
do
{
if ((ByteRecived = ClientSocket1->Socket->ReceiveBuf(Buf, Size)) <= 0) break;
MyBuffer += AnsiString(Buf, ByteRecived);
Size -= ByteRecived;
}
while (Size > 0);
/*
for(int i = 1; i <= MyBuffer.Length(); i++)
fprintf(fp,"%c", MyBuffer[i]);
*/
fwrite(MyBuffer.c_str(), 1, MyBuffer.Length(), fp);
delete [] Buf;
}
Code: Select all
void __fastcall TForm1::ClientSocket1Read(TObject *Sender, TCustomWinSocket *Socket)
{
int ByteRecived;
int Size = Socket->ReceiveLength();
if (Size <= 0) return;
char *Buf = new char[Size];
do
{
if ((ByteRecived = ClientSocket1->Socket->ReceiveBuf(Buf, Size)) <= 0) break;
/*
for(int i = 0; i < ByteRecived; i++)
fprintf(fp, "%c", Buf[i]);
*/
fwrite(Buf, 1, ByteRecived, fp);
Size -= ByteRecived;
}
while (Size > 0);
delete [] Buf;
}
Alternatively, I would suggest some further tweaks to both examples to make more effective use of Buf:
Code: Select all
void __fastcall TForm1::ClientSocket1Read(TObject *Sender, TCustomWinSocket *Socket)
{
int ByteRecived;
int SizeToRead = Socket->ReceiveLength();
if (SizeToRead <= 0) return;
char *Buf = new char[SizeToRead];
char *pBuf = Buf;
int SizeInBuf = 0;
do
{
if ((ByteRecived = ClientSocket1->Socket->ReceiveBuf(pBuf, SizeToRead)) <= 0) break;
SizeInBuf += ByteRecived;
SizeToRead -= ByteRecived;
}
while (SizeToRead > 0);
/*
for(int i = 0; i < SizeInBuf; i++)
fprintf(fp,"%c", Buf[i]);
*/
fwrite(Buf, 1, SizeInBuf, fp);
delete [] Buf;
}
Code: Select all
void __fastcall TForm1::ClientSocket1Read(TObject *Sender, TCustomWinSocket *Socket)
{
int ByteRecived;
int SizeToRead = Socket->ReceiveLength();
if (SizeToRead <= 0) return;
int BufSize = min(SizeToRead, 1024);
char *Buf = new char[BufSize];
do
{
if ((ByteRecived = ClientSocket1->Socket->ReceiveBuf(Buf, min(SizeToRead, BufSize))) <= 0) break;
/*
for(int i = 0; i < ByteRecived; i++)
fprintf(fp, "%c", Buf[i]);
*/
fwrite(Buf, 1, ByteRecived, fp);
SizeToRead -= ByteRecived;
}
while (SizeToRead > 0);
delete [] Buf;
}
Now, with that said, why are you using C style file I/O at all, instead of using C++ style file I/O from the C++ standard library (std::ofstream) or C++Builder's RTL (TFileStream, TStreamWriter, etc)?
Lastly, there is simply no structure to your communications at all. The kind of code you have written so far will only work if the server is dumping a file as-is to the connection as soon as the client connects to the server, and then the server disconnects the client after sending the file. But TCP protocols are rarely ever that simple. They have structure to their messages (how does a message start? How does it end? What kind of data fields does a message contain? Are they fixed-length or variable-length? How many bytes are in each field? In what formats?). Commands and responses. Etc. You don't have ANY of that in your code yet. But you need that if you want to be effective at network programming.
For example, when sending a file, you should send the file's total size as a fixed-sized integer (preferably as a 64bit integer, unless 32bit suits your needs) in network byte order, and optionally also its name as a length-prefixed UTF-8 string, before sending the actual file data. That way, the receiver can know how many bytes to expect up front so it knows when to stop writing bytes to its local file, and optionally knows what to name its local file.
There is a lot of things you have to handle when doing network programming. And you are having to do all of them manually since you are coding (almost) directly to the TCP socket, instead of using a higher-level wrapper that handles these kind of details for you. TClientSocket is very bare-bones, consider switching to a better socket component instead, such as TIdTCPClient in Indy, which has many methods available for reading/sending integers, strings, streams, etc.
