0

This is my sender code snippet.

if(ThreadQ.try_dequeue(temp)){
            if(seqno>=2147483645)
            {
                seqno=-1;
            }
            if(frameno>=29)
            {
                frameno=-1;
            }
            seqno++;
            frameno++;
            fragno=0;
            std::ofstream f1("packet.txt",std::ios::app);
            for(int j=0;j<5;j++)
            {
                //Packetize-Fragment
                fp.fragno=j;
                fp.pl.line[0]=temp.line[k++];
                fp.pl.line[1]=temp.line[k++];
                fp.pl.line[2]=temp.line[k++];
                fp.pl.line[3]=temp.line[k++];
                fp.seqno = seqno;
                fp.frameno = frameno;                           
                retval = send(conn_socket, (char *)&fp, sizeof(fp), 0);
                for (i = 0; i < 4; i++)
                {
                    f1 << fp.seqno << " " << fp.frameno << " " << fp.fragno << " " << fp.pl.line[i].x << " " << fp.pl.line[i].y << " " << fp.pl.line[i].z << " " << fp.pl.line[i].ch << "\n";                   
                }
            }
            f1 << "\n\n";
            k=0;
 }

and these are the relevant structures,

    typedef struct PacketPos{
        float x;
        float y;
        float z;
        int ch;     
    };

    typedef struct PacketPL2{
        PacketPos line[4];
    };
    typedef struct FinalPacket{
        PacketPL2 pl;
        int seqno;
        int frameno;
        int fragno;     
    };

But when I receive it at the receiver end, over UDP (Receiver code shown below):

char * Buffer = (char *)malloc(1000);
while (1){
        retval = recvfrom(msgsock, Buffer, 10000, 0, (struct sockaddr *)&from, &fromlen);
        printf("%d ", retval);
        fp = *(FinalPacket*)Buffer;
        std::ofstream fout("output.txt", std::ios::app);

        for (int i = 0; i < 4; i++)
        {
            fout << fp.seqno << " " << fp.frameno << " " << fp.fragno << " " << fp.pl.line[i].x << " " << fp.pl.line[i].y << " " << fp.pl.line[i].z << " " << fp.pl.line[i].ch;
            fout << "\n";
        }
        fout << "\n\n";     
    }

the float data is not received and I just see 0s in the place of the float data. I'm a beginner, so can anyone tell me what I'm doing wrong here? Thanks in advance.

shreyashk
  • 97
  • 7

3 Answers3

1

I don't know the architecture where you are running. I suppose it is x86 or 64 bits. The snippet you show is incomplete and there is at least one coding error. First error, line is a vector of 4 elements:

typedef struct PacketPL2 {
    PacketPos line[4];
};

In the client:

fp.pl.line[0]=temp.line[k++];

k at some moment will be greater than 3 and you have a buffer overflow because you are setting k to 0 outside the loop.

I suppose conn_socket is already connected to the server, is it correct? Otherwise, there is another error.

Other than this, your code should work alright.

VERY IMPORTANT: YOUR CODE IS NOT PORTABLE AT ALL. You must not just cast structures to buffers (and the other way around) if you want to make it portable. I'm talking about portability among different architectures: different int/float/double size and different endianship.

For making it portable you need to define some endianship, floating point representation, and data size for your protocol. Then make each conversion one piece of data at the time. Using #pragma pack will help you only with data alignment in the structure but at the same time, not only it is compiler dependent but also is less efficient for the processor.

I implemented a UDP client-server with your code (but using sendto in the client), and except for the error above, it works OK. The code is not nice, I tried to put your snippets inside but it works.

Client:

typedef struct PacketPL2
{
    PacketPos line[4];
} s_pp2;
typedef struct FinalPacket
{
    PacketPL2 pl;
    int seqno;
    int frameno;
    int fragno;     
} s_fp;

int main()
{
    int seqno = 214000098;
    int frameno = 10;
    seqno++;
    frameno++;
    int fragno=0;
    s_fp fp;
    s_pp2 temp;
    int conn_socket;
    struct sockaddr_in servaddr;

    temp.line[0].x = 4.56;
    temp.line[0].z = 3.56;
    temp.line[1].x = 7.99;
    temp.line[1].z = 5.99;
    temp.line[2].x = 3.99;
    temp.line[2].z = 4.59;
    temp.line[3].x = 1.51;
    temp.line[3].z = 2.33;

    bzero(&servaddr,sizeof(servaddr));
    servaddr.sin_family = AF_INET;
    servaddr.sin_addr.s_addr=inet_addr("127.0.0.1");
    servaddr.sin_port=htons(32000);

    conn_socket = socket(AF_INET, SOCK_DGRAM, 0);

    using namespace std;

    int k = 0;

    for(int j=0;j<5;j++)
    {
        //Packetize-Fragment
        fp.fragno=j;
        //ERROR, buffer overflow: WHEN K > 3 
        fp.pl.line[0]=temp.line[k++];
        fp.pl.line[1]=temp.line[k++];
        fp.pl.line[2]=temp.line[k++];
        fp.pl.line[3]=temp.line[k++];
        fp.seqno = seqno;
        fp.frameno = frameno;                           

//        int retval = send(conn_socket, (char *)&fp, sizeof(fp), 0);
        int retval = sendto(conn_socket,(char *)&fp, sizeof(fp),0, (struct sockaddr *)&servaddr,sizeof(servaddr));        

        cout << "RETVAL cli:" << retval << endl;

        for (int i = 0; i < 4; i++)
        {
            cout << fp.seqno << " " << fp.frameno << " " << fp.fragno << " " << fp.pl.line[i].x << " " << fp.pl.line[i].y << " " << fp.pl.line[i].z << " " << fp.pl.line[i].ch << "\n";                   
        }
    }

    cout << "\n\n";

    //K IS INITIALIZED HERE
    k=0;

    return 0;
}

Server:

#include <iostream>
#include <sys/types.h>
#include <sys/socket.h>
#include <net/if.h>
#include <netinet/in.h>
#include <stdlib.h>
#include <strings.h>

typedef struct PacketPos
{
    float x;
    float y;
    float z;
    int ch;     
} s_pp;

typedef struct PacketPL2
{
    PacketPos line[4];
} s_pp2;
typedef struct FinalPacket
{
    PacketPL2 pl;
    int seqno;
    int frameno;
    int fragno;     
} s_fp;

int main()
{
    char * Buffer = (char *)malloc(1000);
    int msgsock;
    s_fp fp;
    struct sockaddr_in servaddr, from;
    socklen_t fromlen;

    bzero(&from, sizeof(from));
    bzero(&servaddr, sizeof(servaddr));

    servaddr.sin_family = AF_INET;
    servaddr.sin_addr.s_addr=htonl(INADDR_ANY);
    servaddr.sin_port=htons(32000);


    msgsock = socket(AF_INET, SOCK_DGRAM, 0);
    bind(msgsock,(struct sockaddr *)&servaddr,sizeof(servaddr));


    using namespace std;

    while (1) 
    {
        int retval = recvfrom(msgsock, Buffer, 10000, 0, (struct sockaddr *)&from, &fromlen);
        cout << "RETVAL:" << retval << endl;

        fp = *(FinalPacket*)Buffer;

        for (int i = 0; i < 4; i++)
        {
            cout << fp.seqno << " " << fp.frameno << " " << fp.fragno << " " << fp.pl.line[i].x << " " << fp.pl.line[i].y << " " << fp.pl.line[i].z << " " << fp.pl.line[i].ch;
            cout << "\n";
        }
        cout << "\n\n";     
    }

    return 0;
}

See these links for floating point representation and size:

What is the size of float and double in C and C++?

Fixed-size floating point types

Community
  • 1
  • 1
rodolk
  • 5,606
  • 3
  • 28
  • 34
  • Thanks a lot! The temp is actually a different struct, with line[20], so I've taken care that it does not overflow. Sorry for not mentioning it in the question. I'm still new to this, so I don't know much about portability. Since this is a small application, run on two systems in the same LAN, I didn't think that was necessary. I'll try to incorporate it now, though. Thanks! – shreyashk Jun 26 '15 at 09:49
0

In your code : fp = *(FinalPacket*)Buffer will not be casted to Final Packet because sizeof(FinalPacket) is NOT what you expect.

For example:

Let's say we have a struct:

struct Point
{
 int x;
 int y;
}

Then sizeof(Point) is not 2 * sizeof(int) because of padding involved. Google for further info.

The solution to this is to use pragma pack

So in your case, you should surround your struct with pragma pack.

Example:

#pragma pack(push, 1)
typedef struct FinalPacket{
        PacketPL2 pl;
        int seqno;
        int frameno;
        int fragno;     
    };
#pragma pack(pop)

Now you will be able to cast the buffer directly to struct.

bhavesh
  • 1,343
  • 2
  • 14
  • 23
  • You should note that "pragma pack" is possibly compiler-specific. A few ways of packing structs are part of this question: http://stackoverflow.com/questions/1537964/visual-c-equivalent-of-gccs-attribute-packed – FourtyTwo Jun 25 '15 at 07:46
0

Your question is easy to solve. I have wrote a simple test for udp communication. Now I give your my codes, only key points:

//my struct: 
struct TestCase
{
   float x;
   float y;
};

//client key points:
TestCase case_2;
case_2.x = 0.5;
case_2.y = 1.0;
if(-1 == sendto(sk_fd, (char*)&case_2, sizeof(case_2), 0, (struct sockaddr*)&remote, sizeof(remote)))
{
    cout << "client send data failed, error is " << strerror(errno) << endl;
    return 0;
}

//server key points:
TestCase server;
while(1)
{
    struct sockaddr_in client;
    memset(&server, 0, sizeof(server));
    socklen_t client_len = sizeof(client);
    const int result = recvfrom(sk_fd, &server, sizeof(server), 0, (struct sockaddr*)&client, &client_len);
    if(result < 0)
        cout << "server recv error is " << strerror(errno) << endl;
    cout << server.x << ' ' << server.y << endl;
    break;
}

After you see these above, I think you can know well. You only need to change your code: char * Buffer = (char *)malloc(1000). You should use the struct for receiving the data. Now Do you see it ? I hope this can help you.

cwfighter
  • 502
  • 1
  • 5
  • 20
  • Can you describe how your approach differs from the code that was posted in the question? – Jeremy Friesner Jun 25 '15 at 14:15
  • @JeremyFriesner, the question send to a struct, but it use a `char* buffer` for recving. My solution is that I use a same struct for receiving, so I can get the float. The code `TestCase server; const int result = recvfrom(sk_fd, &server, sizeof(server), 0, (struct sockaddr*)&client, &client_len);`. – cwfighter Jun 26 '15 at 01:58