Getting len of buffer for recv function

Question

I wrote a simple FTP application that can send files back and forth between a client and a server and it was working fine. More recently I wrote a socket library to use with the client and server, some of the functionality has changed, and I'm having trouble wrapping my head around how to get it to work. At the moment I'm integrating the library on my server.

The issue is that a part of my specs are to hide the socket handle with the library, so I've wrapped 'recv' and 'send' in library functions that pass a char pointer by reference. Before I implemented this I was passing char[] directly into the recv functions which was coming out nicely enough for my purposes. Now that I'm using a char* it would seem like I need to know the exact length of the incoming message as my char* is coming out with the send data as well as garbage characters.

Here is a part of my server code:

while (true)
{       
    command = (char*)malloc(sizeof(char)*32);
    int bytesRecv = socketObject.receiveData('c', &command, 32);        

    if(_stricmp(command,"mput") == 0)
    {
        while( true ) {

             SizeCheck = 0;
             FileSize = 0;

             fileName = (char*)malloc(sizeof(char)*1024);                

             bytesRecv = socketObject.receiveData('c', &fileName, 1024);

             if(_stricmp(fileName,"goodbye") == 0)
             {                      
                break;
             }

            while( true )
            {
                char GotFileSize[1024];
                GotFileSize = (char*)malloc(sizeof(char)*1024); 
                socketObject.receiveData('c', &sentSize, 1024);


                FileSize = atoi(GotFileSize);

                if (FileSize > 0)
                {   
                    break;
                }
            }

            mfcc = (char*)malloc(sizeof(char)*FileSize);
            FILE *fp;
            if( (fp = fopen(fileArray, "wb")) == NULL)
            {
                std::cout << "fopen has caused an error" << endl;
            }

            while(SizeCheck < FileSize){
                int Received = socketObject.receiveData('f', &mfcc, FileSize);              

                if(Received != SOCKET_ERROR)
                {   
                    std::cout << "Before fwrite" << std::endl;
                    int written = fwrite(mfcc, 1, FileSize, fp);
                    SizeCheck += Received;
                    fflush(fp);
                }   
            }//while transfer
        }//while true
    }//end mput if

Here is my receive function:

int Socket::receiveData( char socketType, char** data, int size)
{
if(socketType != 'f' && socketType != 'c')
{
    return -1;
}
if(socketType == 'f')
{
    int bytes = recv( fAccepted, *data, size, 0);
    return bytes;
}
else if(socketType == 'c')
{   
    int bytes = recv( cAccepted, *data, size, 0);
    if (bytes == SOCKET_ERROR) {
        printf("send failed: %d\n", WSAGetLastError());
    }
    return bytes;
}

return -1;
}

I've done some reading up on recv that tells me I should somehow either send the size of the filename with the filename, or compile a full string in a loop. I'm not sure if these methods are appropriate for what I'm attempting to do, or if there is an easier way.

Answer 1

The receiveData function is perfectly fine: It writes the received bytes to the buffer, and returns the number of bytes that were received.

All other bytes in the buffer can and should be ignored. In your current code, each time you receive data, you're writing the entire buffer to the file, even though receiveData tells you precisely how much data you should write.

Namely, you shouldn't do

int written = fwrite(mfcc, 1, FileSize, fp);

but instead

int written = fwrite(mfcc, 1, Received, fp);

You should consider using a more reasonable buffer size, such as 1500 bytes (the usual MTU for network packets), or 1MB (something that should fit into the RAM without issues), instead of the full filesize.

By the way, there is no need to pass data as a double pointer, or, as you call it, as a reference to a pointer. Just pass it as a normal pointer. But that has nothing to do with your 'garbage data' issue.