CODEWITHSUNDEEP
csockets
dsell002
dsell002

Reputation: 1306

C error reading packet from streaming socket

I'm just playing around with streaming sockets in C, but I'm having an issue reading the data packet returned from a server application. The code below shows the structure which is used on the client and server side.

struct packet
{
    uint16_t f1;
    uint16_t f2;
    uint32_t f3;
    uint16_t pf1;
    uint32_t pf2;
};

Server side for sending:

char buffer[14];
struct packet myPacket;

myPacket.f1 = 2321;
myPacket.f2 = 4423;
myPacket.f3 = 2134;
myPacket.pf1 = 765;
myPacket.pf2 = 9867;

htonPacket(myPacket, buffer);

Function for packing data into buffer.

void htonPacket(struct packet h, char buffer[14]) {
    uint16_t u16;
    uint32_t u32;

    u16 = htons(h.f1);
    memcpy(buffer+0, &u16, 2);

    u16 = htons(h.f2);
    memcpy(buffer+2, &u16, 2);

    u32 = htonl(h.f3);
    memcpy(buffer+4, &u32, 4);

    u16 = htons(h.pf1);
    memcpy(buffer+8, &u16, 2);

    u32 = htonl(h.pf2);
    memcpy(buffer+10, &u32, 4);
}

Client side for unpacking and printing:

void ntohPacket(struct packet* h, char buffer[14]) {
    uint16_t u16;
    uint32_t u32;

    memcpy(&u16, buffer+0, 2);
    h->f1 = ntohs(u16);

    memcpy(&u16, buffer+2, 2);
    h->f2 = ntohs(u16);

    memcpy(&u32, buffer+4, 4);
    h->f3 = ntohl(u32);

    memcpy(&u16, buffer+6, 2);
    h->pf1 = ntohs(u16);

    memcpy(&u32, buffer+8, 4);
    h->pf2 = ntohl(u32);
}

Printing unpacked data:

printf("myPacket.f1: %d\n", myPacket.f1);
printf("myPacket.f2: %d\n", myPacket.f2);
printf("myPacket.f3: %d\n", myPacket.f3);
printf("myPacket.pf1: %d\n", myPacket.pf1);
printf("myPacket.pf2: %d\n", myPacket.pf2);

When I print the values, it becomes apparent that I've had some issue either addressing or writing to the wrong memory location, but I can't seem to find the error. 

myPacket.f1: 2321
myPacket.f2: 4423
myPacket.f3: 2134
myPacket.pf1: 2134
myPacket.pf2: 50135040

Upvotes: 0

Views: 408

Answers (3)

vonbrand
vonbrand

Reputation: 11791

Why all the dance with copying stuff from one struct into a variable, then adjust endianness, and memcpy? You could just do dst->f1 = htons(src->f1) in one go. It is much easier to just have structs to handle stuff, not fool around with individual bytes (sure, you have to be careful that the compiler doesn't sneak padding in). I think that redoing your code that way will fix your problem.

Upvotes: 1

nneonneo
nneonneo

Reputation: 179422

Well, you are using different offsets for your memcpy operations, so of course you get garbage...

memcpy(buffer+0, &u16, 2);
memcpy(buffer+2, &u16, 2);
memcpy(buffer+4, &u32, 4);
memcpy(buffer+8, &u16, 2);
memcpy(buffer+10, &u32, 4);

vs.

memcpy(&u16, buffer+0, 2);
memcpy(&u16, buffer+2, 2);
memcpy(&u32, buffer+4, 4);
memcpy(&u16, buffer+6, 2);
memcpy(&u32, buffer+8, 4);

Your last lines in ntohPacket should read

memcpy(&u16, buffer+8, 2);
h->pf1 = ntohs(u16);

memcpy(&u32, buffer+10, 4);
h->pf2 = ntohl(u32);

Upvotes: 2

PQuinn
PQuinn

Reputation: 1022

Your memcpy offsets are wrong. Fixed:

memcpy(&u16, buffer+0, 2);
h->f1 = ntohs(u16);

memcpy(&u16, buffer+2, 2);
h->f2 = ntohs(u16);

memcpy(&u32, buffer+4, 4);
h->f3 = ntohl(u32);

memcpy(&u16, buffer+8, 2);  <-- here
h->pf1 = ntohs(u16);

memcpy(&u32, buffer+10, 4);  <-- here
h->pf2 = ntohl(u32);

Upvotes: 1

Related Questions