Transfer array of integers over a socket in C/C++

Question

This answer shows methods for sending and receiving integers over sockets in c/c++. Based on that I wrote following two functions, sendKInt and receiveKInt to send and receive K integers. But its not working correctly. Maybe my pointer arithmetic is wrong, but I cannot find the error. Please help with that, I am new to c++. Also see sendTill and receiveTill functions.

bool sendKInt(int fd, int num[], int k){ // returns true on success
    int32_t conv[k];
    char *data;
    for (int i=0; i<k; i++){
        conv[i] = htonl(num[i]);
        char *data = (char*)conv;
    }
    return sendTill(sizeof(int32_t)*k, fd, data);
}

bool receiveKInt(int fd, int *num, int k){ // returns true on success
    int32_t ret[k];
    int toReturn = receiveTill(sizeof(int32_t)*k,fd,(char*)ret);
    for (int i=0; i<k; i++){
        num[i] = ntohl(ret[i]);
    }
    return toReturn;
}

bool sendTill(int size, int fd, char* dataU){ // dataU - dataUser pointer is NULL then create your own buffer and send random thing // returns true on successs
    char *data;
    int left = size;
    int rc;
    if(dataU == NULL){
        data = (char*)malloc(size);
    }else{
        data = dataU;
    }
    while (left) {
        rc = write(fd, data + size - left, left);
        if (rc < 0){
            cout <<"-1 in sendTill \n";
            return false;
        }
        left -= rc;
    }
    return true;
}

bool receiveTill(int size, int fd, char* data){ // just give a char pointer after allocating size // returns true on success
    int ret;
    int left = size;
    while (left) {
        ret = read(fd, data + size - left, left);
        if (ret < 0){
            cout <<"-1 in receiveTill \n";
            return false;
        }
        left -= ret;
    }
    return true;
}

Answer 1

A nicely working code:

bool sendKInt(int fd, int num[], int k){ // returns true on success
    std::vector<int32_t> conv(k);
    for (int i=0; i<k; i++){
        conv[i] = htonl(num[i]);
    }
    return sendTill(sizeof(int32_t)*k, fd, (char *)conv.data());
}

bool receiveKInt(int fd, int *num, int k){ // returns true on success
    int32_t *ret = (int32_t*)calloc (sizeof(int32_t), k);
    bool toReturn = receiveTill(sizeof(int32_t)*k,fd,(char*)ret);
    for (int i=0; i<k; i++){
        num[i] = ntohl(ret[i]);
    }
    free(ret);
    return toReturn;
}

bool sendTill(int size, int fd, char* dataU){ // dataU - dataUser pointer is NULL then create your own buffer and send random thing // returns true on successs
    char *data;
    int left = size;
    int rc;
    if(dataU == NULL){
        data = (char*)malloc(size);
    }else{
        data = dataU;
    }
    while (left) {
        rc = write(fd, data + size - left, left);
        if (rc < 0){
            cout <<"-1 in sendTill \n";
            return false;
        }
        left -= rc;
    }
    return true;
}

bool receiveTill(int size, int fd, char* data){ // just give a char pointer after allocating size // returns true on success
    int ret;
    int left = size;
    while (left) {
        ret = read(fd, data + size - left, left);
        if (ret < 0){
            cout <<"-1 in receiveTill \n";
            return false;
        }
        left -= ret;
    }
    return true;
}

Answer 2

Why it doesn't work

It is not clear if you intend from sendKInt():

• to send all the data via sendTill(), as suggest by the parameter sizeof(int32_t)*k that you use in the call
• to send the data one by one, as your conversion loop converts only a single item

In sendKInt() loop you have two very big mistakes:

• you declare a bloc variable char *data in the loop. This variable hides the outside variable with the same name, and its value is lost as soon as you exit the loop.
• when you exit the loop you call sendTill() using the outside data variable which is still uninitialized. This is undefined behavior guaranteed !
• Note that even if you wouldn't hide the data variable it wouldn't work, because you cast the conv value (i.e. an integer to transmit) into a pointer to a character (e.g. if the interger is 0, you'll use a null poitner). So you would try to send a rogue pointer, so certainly not the value that yo've expected.

How to solve it

I assume C as you use malloc().

bool sendKInt(int fd, int num[], int k){ // returns true on success
    int32_t *data = calloc (sizeof(int32_t), k);  // allocate for everything
    if (data==NULL)                            // allocation failure
        return 0;                               
    for (int i=0; i<k; i++){                   // convert everything 
        data[i] = htonl(num[i]);
    }
    bool rc = sendTill(sizeof(int32_t)*k, fd, (char*)data);
    free(data);                               // malloc=> free (or you'll leak memory) 
    return rc;                            // now that memory is freed, return the result
}

Note that you have to review the rest of the code according to this new logic. Don't forget to free memory that you allocate in a function in order to avoid memory leakage.

Answer 3

This

char *data;

is not the same as

char *data = (char*)conv;

You have two datas, one inside the for loop which is assigned and one outside that is not so

return sendTill(sizeof(int32_t)*k, fd, data);

winds up sending the outer, uninitialized, data. Crom only knows what will happen.

Fortunately, there is no point to doing

char *data = (char*)conv;

a few hundred times, so

return sendTill(sizeof(int32_t)*k, fd, (char*)conv);

looks like it will work. Sadly it won't in C++.

int32_t conv[k];

is invalid C++. k is not known at compile time, so this requires a non-standard Variable Length Array compiler extension. Some compilers will allow this syntax, but it has some downsides (common implementations easily overrun the stack and horribly messes with the behaviour of sizeof) and won't compile on all compilers. Really sucks if you need to port the code to other platforms or the code is being submitted to someone who will evaluate the code and uses, for example, Visual Studio.

C++11 has std::vector::data so you can

bool sendKInt(int fd, int num[], int k){ // returns true on success
    std::vector<int32_t> conv(k);
    for (int i=0; i<k; i++){
        conv[i] = htonl(num[i]);
    }
    return sendTill(sizeof(int32_t)*k, fd, (char *)conv.data());
}

Answer 4

You declare two data variables, the second one hiding the first.

bool sendKInt(int fd, int num[], int k){ // returns true on success
    int32_t conv[k];

    // First "data", points to undefined value
    char *data;
    for (int i=0; i<k; i++){
        conv[i] = htonl(num[i]);

        // local data, points to conv
        // HIDES the first data declared above
        char *data = (char*)conv;
    }

    // uses the only "data" in scope, which points to undefined location
    return sendTill(sizeof(int32_t)*k, fd, data);
}