tags:

views:

683

answers:

3

Hello,

I'm trying to send a string to a server application using C, but I've hit a snag. I'm fairly new to network programming, and I think my code is barking up the wrong tree.

The message is supposed to be message length + message and is unpacked on the other side by a python server as such (buf being the raw incoming data):

msg_len_bytes = buf[0:4]
msg_len = struct.unpack("!L", msg_len_bytes)[0]

! means network byte order and L means unsigned long.

It is fairly simple to send a regular string. send(sock, message, strlen(message), 0);

But adding the message length I can't quite get a handle on. Here is the code for my client thus far:

struct msgstruct {
        uint32_t length;
        char send_data[4096];
};

int main()

{
    int sock;
    struct msgstruct message;
    char data[4096] = "<MOP><test/></MOP>";

    for ( int i = 0; i < strlen(data); i++ ) {
      message.send_data[i] = data[1];
    }

    struct hostent *host;
    struct sockaddr_in server_addr;

    unsigned long buflen = sizeof(message.send_data);
    uint32_t bufsend = htonl(buflen);

    message.length = bufsend;

    host = gethostbyname("127.0.0.1");

    if ((sock = socket(AF_INET, SOCK_STREAM, 0)) == -1) {
     perror("Socket");
     exit(1);
    }

    server_addr.sin_family = AF_INET;     
    server_addr.sin_port = htons(12998);   
    server_addr.sin_addr = *((struct in_addr *)host->h_addr);
    bzero(&(server_addr.sin_zero),8); 


    if (connect(sock, (struct sockaddr *)&server_addr,
       sizeof(struct sockaddr)) == -1) {
     perror("Connect");
     exit(1);
    }

    if(send(sock, message.length + message.send_data, sizeof(message), 0) == -1){
     printf("\nSocket error.");
     exit(1);
    }
    return 0;
}

I've tried a few variations, but I always end up with a socket error. Is it because I'm mixing types in the second arguement for send? I get compilation errors if I try to send the struct.

+3  A: 

The error is in this code:

send(sock, message.length + message.send_data, sizeof(message), 0)

The Prototype for send is:

ssize_t send(int s, const void *buf, size_t len, int flags);

Note that parameter 2 is a pointer. In your code, you've given it as a length (Type uint32_t) added to a buffer (Type char*). This addition will result in a char*, (pointer-to-char) but a pointer to an unpredictable and meaningless area of memory.

To get a pointer to the buffer, you want:

send(sock, &message, sizeof(message), 0)

Note that taking the address of a struct isn't portable or always advisable, due to padding issues. But on a typical 32-bit architecture, this should be fine.

This will send data starting at the message struct, but sending 4100 (4096+4) bytes!. I don't think you intend to send that much. The 3rd parameter says how many bytes to send, and should be set to:

sizeof(uint32_t) + strlen(data);  // 4-byte Integer + Length of the data "<MOP><test/></MOP>"

Note that this does not include a Null-Terminator for the data, but that your initial for-loop didn't copy a Null-Terminator either

(If you want the null-terminator, make your initial for-loop go to strlen(data)+1, and use strlen(data)+1 in other places as well).

Ideally, you should cache strlen(data) to a local variable, and not call it so much. (you also repeatedly call strlen in the inital for-loop).

Your final statement will look like:

if(send(sock, &message, sizeof(uint32_t)+strlen(data), 0) == -1){
    printf("\nSocket error.");
    exit(1);
}

Try that, and let me know how it goes.

abelenky
Hmm, when I do that, it seems to send a large amount of rubbish to the server (and drives my PC speaker haywire).
What unwind said as well. You have a data[1] where you should have data[i]!
abelenky
sorry, commented on the wrong post
You are quite correct abelenky, thanks! That's quite an embarrassing mistake. Now the server print a couple bytes of junk (the message length I hope), the message itself (where it was MMMMMMMMMMM before), and then a few k of seemingly randombits (am I not null terminating? or should I set the char buffer to be equal to the length of the string?).
Thanks abelenky! That seems to send the correct message. For some reason it won't count the message as received unless I send at least 1024 bytes of data, but I'll just assume that's a problem somewhere in the server's code.
directedition
@directedition: Seems to be Nagle's algorithm preventing you from sending small packets -- try to disable it (example in my answer).
Mihail
A: 

The loop that copies data into the message instance is broken, it has a 1 where it should have an i.

The second argument to send() should be a pointer to the first byte to send. You are instead giving it the sum of a big-endian number (which, if your platform is not big-endian, will be very wrong) and a random array base address. This is wrong.

You need:

if(send(sock, &message, sizeof message.length + strlen(data), 0) == -1) {

There is also some other confusion, you seem to "want" to always send the full 4K, rather than just the amount necessary.

unwind
+3  A: 

You can use 2 subsequent sends:

send(sock, &message.length, sizeof(message.length), 0);
send(sock, message.send_data, message.length*sizeof(char), 0);

Or better prepare buffer with first 4 bytes as message length:

char buff[MAX_BUFF] = "";
int  len_disp = sizeof(message.length);
memcpy(buff, &message.length, len_disp);
memcpy(&buff[len_disp], &message.length, message.length*sizeof(char));
send(sock, buff, message.length*sizeof(char) + len_disp, 0);

EDIT: For small messages comment -- disabling Nagle's algorithm.

BOOL bNagleEnabled = FALSE;
if(setsockopt(sAccept, IPPROTO_TCP, TCP_NODELAY, (char *)&bNagleEnabled, sizeof(BOOL)))
{
  ReportError("Setting TCP_NODELAY socket option failed");
  return -2;
}
Mihail
+1 on using separate sends for length and data. This is particularly important if the compiler feels like inserting data between the length and buffer.
D.Shawley