views:

203

answers:

6

I posted on this topic earlier but now I have a more specific question/problem.

Here is my code:

#include <cstdlib>
#include <iostream>

typedef struct{
    unsigned int h;
    unsigned int b[];
    unsigned int t;
 } pkt;

 int main(){

    unsigned int* arr = (unsigned int*) malloc(sizeof(int) * 10);
    arr[0] = 0xafbb0000;
    arr[1] = 0xafbb0001;
    arr[2] = 0xafbb0011;
    arr[3] = 0xafbb0111;
    arr[4] = 0xafbb1111;
    arr[5] = 0xafbc0000;
    arr[6] = 0xafbc0001;
    arr[7] = 0xafbc0011;
    arr[8] = 0xafbc0111;
    arr[9] = 0xafbc1111;

    pkt* p = (pkt*) malloc(sizeof(int)*13);
    p->h = 0x0905006a;

    int counter;

Here's what I get for(counter=0; counter < 10; counter++) p->b[counter] = arr[counter];

    p->t = 0x55555555;

    std::cout << "header is \n" << p->h << std::endl;
    std::cout << "body is" << std::endl;
    for(counter=0; counter < 10;++counter)
            std::cout << std::hex << *((p->b)+counter) << std::endl;
    std::cout << "trailer is \n" << p->t << std::endl;

}

Here's what I get

header is 
151322730
body is
55555555
afbb0001
afbb0011
afbb0111
afbb1111
afbc0000
afbc0001
afbc0011
afbc0111
afbc1111
trailer is 
55555555

*(p->b) is replaced with the trailer! And if I remove the line where I assigned the trailer, that is p->t=0x55555555;, then the trailer and p->b are the same (afbb0000).

So my question is, how can I keep the structure define at the top the way it is and have this packet function properly.

EDIT:

I should be more clear. I writing this for a networking application, so I have to make a packet where the output is in this exact order, I am testing this by writing to a file and doing a hex dump. So my question really is:

How do I make a packet with a header, variable sized body, and a trailer always have this order? The only two solutions I can think of are having many different structures, or having a vector.

That is, I could structs where

typedef struct{
        unsigned int h;
        unsigned int b[12];
        unsigned int t;
     } pkt1;

typedef struct{
        unsigned int h;
        unsigned int b[102];
        unsigned int t;
     } pkt2;

etc

or I could do

std::vector<unsigned int> pkt (12);
pkt[0] = header;
pkt[1] = data;
...
pkt[2]= data;
pkt[11] = trailer;

I don't really like either of these solutions though. Is there a better way??

Also, this is somewhat of a separate question, I will have to do the same thing for receiving data. It is wise to do something like cast a block of data to a vector? I am going to be receiving the data as a void* and I will know the max length.

+4  A: 

Basically, you can't. I'm surprised that this even compiles. You're only allowed to have an array without a specified size as the last element of a struct, because of the fundamental way that structs work in C/C++.

The problem is that the compiler has to know how big b[] is in order to know where to find t in the struct. Normally it does this by looking at the array size. Since you didn't pass a size, your compiler evidently treats it as zero, which means that b[] and t are actually at the same place in memory! You need to either specify the size of b[] in the struct declaration, or you need to move t before b[].

JSBangs
Yeah, it shouldn't compile. gcc says: *error: flexible array member not at end of struct*
sth
+1  A: 

If you change

typedef struct{
    unsigned int h;
    unsigned int b[];
    unsigned int t;
} pkt;

To

typedef struct{
    unsigned int h;
    unsigned int *b;
    unsigned int t;
} pkt;

I believe it will work. That way, b and t will each take up their own space, and not the same space. You would also need to change malloc(sizeof(int) * 13); to malloc(sizeof(pkt)); - the size you malloc() will be constant, and you'll need to malloc() twice - once for the struct, and once for the array in b. Of course, you're already doing that anyway, but this way you won't be wasting heap space.

If you want to use b[] instead of *b (and there are valid reasons for this), you will need to put it at the end. Sorry. That's how hacks work.

Chris Lutz
+1  A: 

When you declare unsigned int b[] in the struct it is not allocating any space for it. So pkt.b and pkt.t are the same location when you dereference the struct pointer. You could make b[] the last field in the struct.

However, if you want t to follow b then you could try this C example:

#include <stdio.h>
#include <malloc.h>

#define PKT_STRUCT(size) struct{unsigned int h;unsigned int b[size];unsigned int t;}

int main(void)
{

    PKT_STRUCT(10) *pkt  = malloc(sizeof(PKT_STRUCT(10)));

    pkt->h = 0x0905006a;

    pkt->b[0] = 0xafbb0000;
    pkt->b[1] = 0xafbb0001;
    pkt->b[2] = 0xafbb0011;
    pkt->b[3] = 0xafbb0111;
    pkt->b[4] = 0xafbb1111;
    pkt->b[5] = 0xafbc0000;
    pkt->b[6] = 0xafbc0001;
    pkt->b[7] = 0xafbc0011;
    pkt->b[8] = 0xafbc0111;
    pkt->b[9] = 0xafbc1111;

    pkt->t = 0x55555555;

    printf("Header:  %x\n", pkt->h);

    printf("Body:\n");
    int i;
    for(i=0; i<10; ++i)
     printf("%x\n", pkt->b[i]);

    printf("Tail:  %x\n", pkt->t);


    return 0;
}

This might be useful if you are working with data that is already packed.

Also you could use a union of different packet size / structures.

Karl Voigtland
Doesn't compile. error: types may not be defined in 'sizeof' expressions.
Rev316
+6  A: 

Try updating the struct and then adding the following:

typedef struct{
    unsigned int h;
    unsigned int* b;
    unsigned int t;
 } pkt;

...
...

pkt p;
p->b = arr

Here is a visual example of what it is doing...

|----------------------|
|     header           |
|----------------------|
|     B   int*         |  -------  p->b = arr
|----------------------|        | 
|     trailer          |        |
|----------------------|        v
                              |----------------------| 
                        arr   |                      |
                              |    B items [0 ... N] |
                              |                      |
                              |----------------------|

If you want to get tricky you can do something like this ...

|----------------------|
|     header           |
|----------------------|
|     B   int*         |  -------  (= to memory address after trailer)
|----------------------|        | 
|     trailer          |        |
|----------------------|        |
|                      | <-------
|    B items [0 ... N] |
|                      |
|----------------------|

Here is a working version of the later:

#include "stdlib.h"
#include "stdio.h"
#include "malloc.h"

typedef struct{
    unsigned int h;
    unsigned int* b;
    unsigned int t;
 } pkt;

 int main(){

    pkt* p = (pkt*) malloc( sizeof(pkt) + sizeof(int)*10);
    p->h = 0x0905006a;
    p->b = &(p->t)+1;
    p->t = 0x55555555;

    p->b[0] = 0xafbb0000;
    p->b[1] = 0xafbb0001;
    p->b[2] = 0xafbb0011;
    p->b[3] = 0xafbb0111;
    p->b[4] = 0xafbb1111;
    p->b[5] = 0xafbc0000;
    p->b[6] = 0xafbc0001;
    p->b[7] = 0xafbc0011;
    p->b[8] = 0xafbc0111;
    p->b[9] = 0xafbc1111;

    int counter;
    printf( "header is \n" );
    printf( "%0x\n", p->h);
    printf( "body is\n" );
    for(counter=0; counter < 10;++counter)
            printf( "%0x\n", (p->b)[counter]);
    printf( "trailer is\n" );
    printf( "%0x\n", p->t );
 }
Casey
Gnarly. I like it.
Paul Nathan
I like this too, but because of the application I can't use this approach.
devin
+1  A: 

AS you've tagged the question C++, you could always use vector<int> instead of int []. Better still, make pkt a class and hide the internal representation from the clients. You would then have an accessor for the header and trailer and an indexer for the body.

Skizz
+2  A: 

A C++ solution:

#include <iostream>
using namespace std;

class Packet
{
public:
  Packet (int body_size) :
    m_body_size (body_size)
  {
    m_data = new int [m_body_size + 2];
  }

  ~Packet ()
  {
    delete [] m_data;
    m_data = 0;
  }

  int &Header ()
  {
    return m_data [0];
  }

  int &Trailer ()
  {
    return m_data [m_body_size + 1];
  }

  int Size ()
  {
    return m_body_size;
  }

  int *Data ()
  {
    return m_data;
  }

  int &operator [] (int index)
  {
    return m_data [index + 1];
  }

  //  helper to write class data to an output stream
  friend ostream &operator << (ostream &out, Packet &packet)
  {
    out << "Header = " << packet.Header () << endl;
    out << "Data [" << packet.Size () << "]:" << endl;

    for (int i = 0 ; i < packet.Size () ; ++i)
    {
      out << "  [" << i << "] = 0x" << hex << packet [i] << endl;
    }

    out << "Trailer = " << packet.Trailer () << endl;

    return out;
  }

private:
  int
    m_body_size,
    *m_data;
};

//  simple test function for Packet class
int main ()
{
  Packet
    packet (10);

  packet.Header () = 0x0905006a;
  packet [0] = 0xafbb0000;
  packet [1] = 0xafbb0001;
  packet [2] = 0xafbb0011;
  packet [3] = 0xafbb0111;
  packet [4] = 0xafbb1111;
  packet [5] = 0xafbc0000;
  packet [6] = 0xafbc0001;
  packet [7] = 0xafbc0011;
  packet [8] = 0xafbc0111;
  packet [9] = 0xafbc1111;
  packet.Trailer () = 0x55555555;

  cout << packet;
}

I've not put error checking in or const accessors, you can bounds check the array access. Receiving data is straightforward:

Packet Read (stream in)
{
   get size of packet (exluding header/trailer)
   packet = new Packet (size)
   in.read (packet.Data, size + 2)
   return packet
}

Skizz

Skizz