How can I send all data over a socket?

Question

I am trying to send large amounts of data over a socket, sometimes when I call send (on Windows) it won't send all the data I requested, as expected. So, I wrote a little function that should have solved my problems- but it's causing problems where the data isn't being sent correctly and causing the images to be corrupted. I'm making a simple chat room where you can send images (screenshots) to each other.

Why is my function not working?

How can I make it work?

void _internal_SendFile_alignment_512(SOCKET sock, BYTE *data, DWORD datasize)
{       
    Sock::Packet packet;
    packet.DataSize = datasize;
    packet.PacketType = PACKET_FILETRANSFER_INITIATE;
    DWORD until = datasize / 512;

    send(sock, (const char*)&packet, sizeof(packet), 0);

    unsigned int pos = 0;

    while( pos != datasize )
    {
        pos += send(sock, (char *)(data + pos), datasize - pos, 0);
    }

}

My receive side is:

public override void OnReceiveData(TcpLib.ConnectionState state)
{
    if (state.fileTransfer == true && state.waitingFor > 0)
    {
        byte[] buffer = new byte[state.AvailableData];
        int readBytes = state.Read(buffer, 0, state.AvailableData);
        state.waitingFor -= readBytes;
        state.bw.Write(buffer);
        state.bw.Flush();

        if (state.waitingFor == 0)
        {
            state.bw.Close();
            state.hFile.Close();
            state.fileTransfer = false;
            IPEndPoint ip = state.RemoteEndPoint as IPEndPoint;
            Program.MainForm.log("Ended file transfer with " + ip);
        }
    }
    else if( state.AvailableData > 7)
    {          
        byte[] buffer = new byte[8];
        int readBytes = state.Read(buffer, 0, 8);
        if (readBytes == 8)
        {
            Packet packet = ByteArrayToStructure<Packet>(buffer);
            if (packet.PacketType == PACKET_FILETRANSFER_INITIATE)
            {
                IPEndPoint ip = state.RemoteEndPoint as IPEndPoint;
                String filename = getUniqueFileName("" + ip.Address);
                if (filename == null)
                {
                    Program.MainForm.log("Error getting filename for " + ip);

                    state.EndConnection();
                    return;
                }

                byte[] data = new byte[state.AvailableData];
                readBytes = state.Read(data, 0, state.AvailableData);

                state.waitingFor = packet.DataSize - readBytes;
                state.hFile = new FileStream(filename, FileMode.Append);
                state.bw = new BinaryWriter(state.hFile);
                state.bw.Write(data);
                state.bw.Flush();
                state.fileTransfer = true;
                Program.MainForm.log("Initiated file transfer with " + ip);
            }
        }
    }
}

It receives all the data, when I debug my code and see that send() does not return the total data size (i.e. it has to be called more than once) and the image gets yellow lines or purple lines in it — I suspect there's something wrong with sending the data.

Answer 1

I mis-understood the question and solution intent. Thanks @Remy Lebeau for the comment to clarify that. Based on that, you can write a sendall() function as given in section 7.3 of http://beej.us/guide/bgnet/output/print/bgnet_USLetter.pdf

int sendall(int s, char *buf, int *len)
{
    int total = 0; // how many bytes we've sent
    int bytesleft = *len; // how many we have left to send
    int n = 0;
    while(total < *len) {
       n = send(s, buf+total, bytesleft, 0);
       if (n == -1) { 
           /* print/log error details */
           break;
       }
       total += n;
       bytesleft -= n;
    }
    *len = total; // return number actually sent here
    return n==-1?-1:0; // return -1 on failure, 0 on success
}

Answer 2

You need to check the returnvalue of send(). In particular, you can't simply assume that it is the number of bytes sent, there is also the case that there was an error. Try this instead:

while(datasize != 0)
{
    n = send(...);
    if(n == SOCKET_ERROR)
        throw exception("send() failed with errorcode #" + to_string(WSAGetLastEror()));
    // adjust pointer and remaining number of bytes
    datasize -= n;
    data += n;
}

BTW:

• Make that BYTE const* data, you're not going to modify what it points to.
• The rest of your code seems too complicated, in particular you don't solve things by aligning to magic numbers like 512.