[Okta Webinar] Learn how to a build a cloud-first strategyRegister Now

x
?
Solved

serialization technique.

Posted on 2005-04-25
102
Medium Priority
?
960 Views
Last Modified: 2013-12-14

So I'm perusing a struct that - for the most part - needs improvement with respect to 'things' I've learned (here) when structuring data types.    So I was perusing the the C++ FAQ text (online version: http://www.parashift.com/c++-faq-lite/serialization.html) authored by Marshall Cline, when I discovered ' serialization'.  

On the issue of "[36.2] How do I select the best serialization technique?".   He states:
//////////
1. Decide between human-readable ("text") and non-human-readable ("binary") formats. The tradeoffs are non-trivial. Later FAQs show how to write simple types in text format and how to write simple types in binary format.
2.  Use the least sophisticated solution when the objects to be serialized aren't part of an inheritance hierarchy (that is, when they're all of the same class) and when they don't contain pointers to other objects.
/////////

For my purposes items 1 and 2.  That said, I'd like to create a serialization class that'll handle the struct/object below.


typedef double tag_word_type;
struct t2_data_t
{
  tag_word_type    tag_word;

  bool             alventdc     : 1;
  bool             alvsvld      : 1;
  bool             alvvhat      : 1;
  unsigned int                  : 1;
  bool             alv2cmd      : 1;
  bool             alv2btrk     : 1;
  bool             aldsel       : 1;
  bool             alflnit      : 1;
                                                        
  bool             alvcmd       : 1;
  bool             alvtrk       : 1;
  bool             alesel       : 1;
  bool             alfrnit      : 1;
                                                        
  unsigned int                  : 1;
  bool             alvwepr      : 1;
  unsigned int                  : 1;

  signed int       alvx         :16;
  signed int       alvy         :16;
  signed int       alvz         :16;

  signed int       alvaz        :12;
  unsigned int                  : 4;

  signed int       alvel        :12;
  unsigned int                  : 4;

  signed int       alvrng       :16;

  signed int       alvhat       :16;
  unsigned int     alvtref      :16;

  signed int       alvnb        :16;
  signed int       alvwb        :16;

  short            unused[21];      
};

struct header
{
  unsigned short size;
  unsigned short id;
  unsigned short count;
  unsigned short chksum;
};

struct outgoing_msg  // sent across the network
{
  header  msg_header;
  t2_data_t t2_data;
};
0
Comment
Question by:forums_mp
  • 54
  • 44
  • 3
  • +1
102 Comments
 
LVL 15

Expert Comment

by:efn
ID: 13863560
This is all plain old data, so there is no need to serialize it if it will be read by a compatible program (one that represents the structure in bits the same way).  If you want an incompatible receiving program to be able to decode it, you really have an encoding issue rather than a serialization issue.

That said, could you make your question more explicit?
0
 

Author Comment

by:forums_mp
ID: 13863709

Ok!!  So much for that idea then.

>>>> That said, could you make your question more explicit?

What I and a colleague of mine are faced with is tansmitting data across a pipe (ethernet).   The interface/data looks like whats shown above and what we've observed is the issues with padding that makes things seemingly interesting.

After much investigation/request for guidance (including here) we've discovered that the ideal thing to do is send the data as a stream of bytes.  To do this we need to encode/decode the data on both platforms.

So were opting to do is generate source code that'll encode the data send it.   Receive data decode it.  While it's true that the two platforms currently in use has compatibile representation, we'd like not to take chances given we have two more platforms we haven't investigated for one reason or another.

It seems to me I'm back to where I started.   "copying the struct to the bytestream" and enforcing my own encoding on it.
That said, Illustrate how you'd encode and decode that in a buffer of bytes.

 

0
 
LVL 15

Expert Comment

by:efn
ID: 13863918
There are lots of different ways to encode the data, with different results in a number of dimensions of evaluation, including:

--how much processing time is required to encode and decode
--how much network bandwidth is required (that is, how many bytes are required to transmit a record)
--how easy/hard it is to develop and maintain
--how easy/hard it is for a human to do the encoding/decoding for testing, debugging, or analysis
--how adaptable the representation is to changes in the content of the data structure
--whether the representation conforms to some standard (there are standards for platform-independent encoding)

What do you want to optimize?  Depending on your answer, I may be able to suggest an encoding scheme.
0
Independent Software Vendors: We Want Your Opinion

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

 
LVL 39

Accepted Solution

by:
itsmeandnobodyelse earned 2000 total points
ID: 13865401
Actually, bit members are a poor way to make a struct portable (and of course it has nothing to do with serialization). If one of the members has more than 8 bits, the little-endian and big-endian issue still arises. Also alignment differences can't reliably solved by  bit members.

As I already told you in a previous thread, I would define a transmission format based on the main platform of the application. All other platforms have to transform messages/members from/to the transmission format.

Serialization is more than converting to different platform formats. Serialization could provide an independent transmission and storing format as well as to creating platform dependent objects from the datastream. Look at that:

#include <iostream>
#include <string>
#include <map>
using namespace std;

#include <process.h>

struct DataStream
{
    string classId;
    string buffer;
    int    curPos;
   
    DataStream(string clsId)
        : classId(clsId), curPos(0)
    {
    }
   
    DataStream(char* buf, int size)
        : curPos(0)
    {
        string temp(buf, size);
        int pos = temp.find(':');
        if (pos == string::npos || pos == 0 || pos == size-1)
            return;
        classId = temp.substr(0, pos);
        buffer  = temp.substr(pos+1);
    }
   
    bool isValid() { return curPos <= buffer.length(); }
    operator bool() { return isValid(); }
   
    int getBufferSize() { return isValid()? classId.size() + 1 + buffer.size() : 0; }
   
    int getBuffer(char*& buf, int siz)
    {
        int sizNeeded = getBufferSize();
        int sizClass  = classId.size();
        if (buf == NULL || siz < sizNeeded)
        {
            delete buf;
            buf = new char[sizNeeded];
        }
        memcpy(buf, classId.c_str(), sizClass);
        buf[sizClass] = ':';
        memcpy(&buf[sizClass+1], buffer.c_str(), buffer.size());
        return sizNeeded;
    }
};

template <class T>
DataStream& operator<<(DataStream& ds, const T& t)
{
    if (ds.isValid())
    {
        int curPos   = ds.curPos;
        ds.curPos += sizeof(T);
        ds.buffer.resize(ds.curPos);
        memcpy(&ds.buffer[curPos], &t, sizeof(t));
    }
    return ds;
}

template <class T>
DataStream& operator>>(DataStream& ds, T& t)
{
    if (ds.isValid())
    {
        int curPos   = ds.curPos;
        ds.curPos += sizeof(t);
        if (ds.isValid())
            memcpy(&t, &ds.buffer[curPos], sizeof(t));
    }
    return ds;
}

DataStream& operator<<(DataStream& ds, const string& s)
{
    if (ds)
    {
        ds.buffer += s;
        ds.buffer += '\0';
        ds.curPos  = ds.buffer.size();
    }
    return ds;
}

DataStream& operator>>(DataStream& ds, string& s)
{
    if (ds)
    {
        s          =  &ds.buffer[ds.curPos];
        ds.curPos  += s.size();
        if (ds.curPos < ds.buffer.length() && ds.buffer[ds.curPos] == '\0')
            ds.curPos++;  // add terminating zero
    }
    return ds;
}


class Persistent;

typedef Persistent* (*CreateInstanceFunc)( void );

class Persistent
{
    // factory map is mapping class names to create functions
    static map<string, CreateInstanceFunc> factoryMap;
public:
    virtual string         getClassId() = 0;
    virtual bool           loadFromStream(DataStream& ds) = 0;
    virtual bool           storeToStream(DataStream& ds) = 0;
   
    static Persistent* createInstance(const DataStream& ds)
    {
        map<string, CreateInstanceFunc>::iterator it;
        if ((it = factoryMap.find(ds.classId)) == factoryMap.end())
            return NULL;
       
        return it->second();
    }
   
    static bool registerClass(const string& className, CreateInstanceFunc createFunc)
    {
        map<string, CreateInstanceFunc>::iterator it;
        if ((it = factoryMap.find(className)) != factoryMap.end())
            return false;    // already registered
       
        factoryMap[className] = createFunc;
        return true;
    }
   
    DataStream*            store()
    {
        DataStream* pds = new DataStream(getClassId());
        if (!storeToStream(*pds))
        {
            delete pds;
            return NULL;
        }
        return pds;
    }
};

map<string, CreateInstanceFunc> Persistent::factoryMap;

// independent baseclass derived from Persistent
class MyDataClass : public Persistent
{
    int i;
    double d;
    string  s;
    bool    b;
    char    c;
   
public:
    static Persistent* createMyDataClass() { return new MyDataClass; }
   
    MyDataClass() : i(1), d(2.), s("abc"), b(false), c('X')
    {
        registerClass("MyDataClass", createMyDataClass);
    }
   
   
    string getClassId() { return "MyDataClass"; }
    bool loadFromStream(DataStream& ds)
    {
        ds >> i >> d >> s >> b >> c;
        return ds.isValid();
    }
    bool storeToStream(DataStream& ds)
    {
        ds << i << d << s << b << c;
        return ds.isValid();
    }
};



int main()
{
    MyDataClass mdc;
   
    DataStream* pds = mdc.store();
   
    char* buf = NULL;
    int   siz = 0;
   
    if ((siz = pds->getBuffer(buf, 0)) > 0)
    {
        DataStream  ds(buf, siz);
        if (ds)
        {
            Persistent* pNew = Persistent::createInstance(ds);
            if (pNew)
                cout << "Created new instance of class " << pNew->getClassId() << endl;
            else
                cout << "Creation failed" << endl;
        }
        else
            cout << "Invalid DataStream " << ds.classId << "  current position: " << ds.curPos << endl;
       
    }
    else
        cout << "getBuffer failed" << endl;
   
    return 0;
}


Here a derived class was serialized using struct DataStream.

By overriding operator>> and operator<< functions of struct DataStream you could make platform dependent implementations. Furthermore, you could derive two different classes from baseclass 'MyDataClass', one for the sender application and one for the target application.

class MySenderClass : public MyDataClass
{
public:
      static Persistent* createMySenderClass() { return new MySenderClass(); }
      MySenderClass() { registerClass("MyDataClass", createMySenderClass); }  
      string getClassId() { return "MySenderClass"; }
};

class MyReceiverClass : public MyDataClass
{
public:    
      static Persistent* createMyReceiverClass() { return new MyReceiverClass();
      MyReceiverClass() { registerClass("MyReceiverClass", createMyReceiverClass); }  
      string getClassId() { return "MyReceiverClass"; }
 
};

Note:

- all persistent data member are in baseclass MyDataClass.
- the registerClass call in baseclass MyDataClass must be removed
- on one system you have only one of the classes MySenderClass and MyReceiverClass
- you may have non-persistent data member in the derived classes

Regards, Alex


0
 

Author Comment

by:forums_mp
ID: 13865867

efn, I see some overlap in with regards to some of the information present, more specifically:

[a]--how easy/hard it is to develop and maintain
[b]--how easy/hard it is for a human to do the encoding/decoding for testing, debugging, or analysis
[c]--how adaptable the representation is to changes in the content of the data structure

Now if I had to rank them:  Whew!!   b, a and c perhaps or b, c, a in that order.

Alex:
This is one of the reasons, I'm going nuts with this.  The real issue here is trying to find - some long term (used sparingly) - solution that I could just pick up without having to worry about compiler issues (padding, etc).  

Now get this:  >>>>> Actually, bit members are a poor way to make a struct portable

When I first investigated this.  Here's specifically what I was 'told'.

[QUOTE]
Do not use structures, unions or classes for
direct communications.  Here are some reasons:
1. Padding.  The compiler is allowed to put
    padding (extra bytes) between the members.
    Thus your model may not resemble the real
    data.

2. Endianism (which you have discovered)

3. Type sizes.  Compilers like to make the width
    of an "int" to match the processor's preferred
    size.  If the processor likes 32-bit integers
    then the compiler will set it that way.

The best method is to allocate an unsigned char
buffer large enough to hold the message.  Read the
message into the buffer.  Convert the endianism
of the data if necessary.  Then load each member
of the model structure from the buffer.  Sending
out data is in the reverse order.

The model will be in the processor's native
data types, which will speed up execution.  By
placing data into the buffer, the program explicitly
controls the layout of the messages.  Also, by using
a buffer, one can use the block reads and writes
without any worries of unknown padding creeping in.

Another nice issue is that copying into the buffer
relies on the design of the messaging system.  If
the compiler changes the data type widths or decides
to put in padding into the structure, your program
won't be affected.  In your current system, you are
screwed if the compiler decides to pad the members
or change the size of an integer.

This I have learned after many painful debug sessions
on embedded platforms.
[/QUOTE]

With regards to item 1 in the text.  I thought wow!!  This might suit well.


0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 13866125
I fully agree to all said between [QUOTE] and [/QUOTE]...

Look at struct DataStream above. The operator<< and operator>> functions actually work on a plain char buffer (by means of std::string). So, padding is covered by the solution. Endianess must/could be resolved by providing specific streaming operators for all integer types that have to call htons/htonl/ntohs/ntohl before or after streaming operation, e. g. by that:

struct Int32BigEnd { int i; Int32BigEnd(int in) : i(htonl(in)){} };

// on MAC
DataStream& operator<<(DataStream& ds, int i)
{
   ds << Int32BigEnd(i);   // use template with big-endian struct type
   return ds;
}

Regards, Alex




0
 

Author Comment

by:forums_mp
ID: 13866287

Just so I get my head aroudn this.   Lets map this appraoch into a struct(s) I'm dealing with.  Take a synopsis of a t2_data and t3_data type transmitted across a network.

typedef double tag_word_type;
struct t2_data_t
{
  tag_word_type    tag_word;

  bool             alventdc     : 1;
  bool             alvsvld      : 1;
  bool             alvvhat      : 1;
  unsigned int                  : 1;
};

struct t3_data_t
{
  tag_word_type    tag_word;

  bool             elventdc     : 1;
  bool             elvsvld      : 1;
  bool             elvvhat      : 1;
  unsigned int                  : 1;
};

struct header
{
  unsigned short size;
  unsigned short id;
  unsigned short count;
  unsigned short chksum;
};

struct outgoing_msg  // sent across the network
{
  header  msg_header;
  t2_data_t t2_data;
  t3_data_t t3_data;
};

outgoing mesage is whats 'transmitted' .   How would you formulate this?
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 13866541
>>>> outgoing mesage is whats 'transmitted' .   How would you formulate this?

In case the question is directed to me:

DataStream& operator<<(DataStream& ds, const header& h)
{
     header h1 = h;
#ifdef MAC
     h1.size = ntohs(h.size);
     h1.id = ntohs(h.id);
     h1.count = ntohs(h.count);
     h1.chksum = ntohs(h.chksum);
#endif
     ds << h1;
     return ds;
}

struct outgoing_msg  : public Persistent
{
  header  msg_header;
  t2_data_t t2_data;
  t3_data_t t3_data;

  bool storeToStream(DataStream& ds)
  {
        ds << msg_header << t2_data << t3_data;
        return ds.isValid();
  }
};

Instead of #ifdef MAC you could use a virtual function, that does nothing on PC but makes the htons conversion on MAC.

Regards, Alex



0
 

Author Comment

by:forums_mp
ID: 13867451

Alex,

OK.. I think I see where this is going .. I'll need to wrap my head aroudn this some more. :)  Now there's a WriteBlock member function within QT that I'll use to send the data.  

//// 1
  writeBlock ( const char * data, uint len);

I'm not quite sure I see where I'll take that stream and call WriteBlock on it.  

//// 2
In doing this:  One needs not concern themselves with padding etc.?
ds << msg_header << t2_data << t3_data;
0
 
LVL 22

Expert Comment

by:grg99
ID: 13868035
Since you are just sending a bunch of 1-bit fields, it's a bit of overkill to worry about endinaness and all.

how's about just:

#define  MapOut(x)  ( (x) != 0 ) ? 'T' : 'F';
    char Out[8];
   
Out[0] =  MapOut( alventdc  );
Out[1] =  MapOut( alvsvld    );
Out[2] =  MapOut( alvvhat   );
Out[4] =  MapOut( elventdc );
Out[5] =  MapOut( elvsvld   );
Out[6] =  MapOut( elvvhat   );  
Out[7] =   '\0';

----
And reverse the code on the receiving end.

0
 

Author Comment

by:forums_mp
ID: 13868217
grg99, it's not just a bunch of '1' bits.  See my initial post to get a feel for the EXACT replica of one of the structs.
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 13868623
>>>> writeBlock ( const char * data, uint len);

If you would take struct DataStream from above it is:

    outgoing_msg msg;
    // fill msg data somehow
    ....

    // build data stream by serializing
    DataStream* pds = msg.store();   // call Persistent::store()

    // get buffer from stream    
    char* buf = NULL;
    int      siz = 0;
    if ((siz = pds->getBuffer(buf, 0)) > 0)
    {
         // send buffer
         writeBlock(buf, siz);
         ...

Regards, Alex
0
 

Author Comment

by:forums_mp
ID: 13868794

Alex, couple of things before I go off and get dangerous here.

//// 1
On the receive side.   How would this look.  The readBlock member function looks like so:
  readBlock ( char * data, uint maxlen );

//// 2  How does this piece fit in after the writeBlock above?

        DataStream  ds(buf, siz);
        if (ds)
        {
            Persistent* pNew = Persistent::createInstance(ds);
            if (pNew)
                cout << "Created new instance of class " << pNew->getClassId() << endl;
            else
                cout << "Creation failed" << endl;
        }

//// 3
Whats the story on padding with regard to this.?
ds << msg_header << t2_data << t3_data;
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 13869266
>>>> 1) On the receive side.   How would this look

Assume you have the same classes and headers on the receiver site:

      char data[4096];
      int bytesRead = 0;
      if ((bytesRead = readBlock ( data, sizeof(data)) == ok )
      {
             DataStream ds(buf, bytesRead);
             if (ds)
             {
                   outgoing_msg* pMsg = dynamic_cast<outgoing_msg*>Persistent::createInstance(ds);
                   if (pMsg)
                        ...
         }
      }

You see you could explicitly cast the return value of Persistence::createInstance if you don't need the generic approach via baseclass pointers. You also could add more virtual functions, e. g. to distribute a specific message to a class or to a container (e. g. a queue) where it would be processed further.

Note, the big-endian/little-endian thing was handled very deep in the stream operators. So, the resulting struct already has a correct endian.

>>>> //// 3
>>>> Whats the story on padding with regard to this.?
>>>> ds << msg_header << t2_data << t3_data;

In the stream functions we 'added' the types using memcpy at byte offsets. Therefore padding isn't an issue as the original struct never was used for transmission purposes.

Regards, Alex
0
 
LVL 39

Assisted Solution

by:itsmeandnobodyelse
itsmeandnobodyelse earned 2000 total points
ID: 13869479
struct outgoing_msg  : public Persistent
{
  header  msg_header;
  t2_data_t t2_data;
  t3_data_t t3_data;

  bool storeToStream(DataStream& ds)
  {
        ds << msg_header << t2_data << t3_data;
        return ds.isValid();
  }
};

That struct isn't complete til now. We would need a static create function to register in the static factoryMap. The registration only happens  once, so it could be done in a static member function:

struct outgoing_msg  : public Persistent
{
  header  msg_header;
  t2_data_t t2_dat;
  t3_data_t t3_dat;
  static bool registered;
 
 static Persistent* createMsg() { return new outgoing_msg; }
 outgoing_msg()
 {
    memset(&msg_header, 0, sizeof(header));
    memset(&t2_dat, 0, sizeof(t2_data));
    memset(&t3_dat, 0, sizeof(t3_data));
 }
  string getClassId() { return "outgoing_msg"; }
  bool storeToStream(DataStream& ds)
  {
        ds << msg_header << t2_dat << t3_dat;
        return ds.isValid();
  }
  bool loadFromStream (DataStream& ds)
  {
        ds >> msg_header >> t2_dat >> t3_dat;
        return ds.isValid();
  }
};

bool outgoing_msg::registered = Persistent::registerClass("outgoing_msg", outgoing_msg::createMsg);

That static member automatically registers the class at program initialisation.

BTW, if you would make a baseclass named transmission_msg and derived classes outgoing_msg and incoming_msg, you could make different implementations of virtual functions, say overrides of Persistent::execute(), on the sender site and on the receiver site. The registered class would be 'transmission_msg' but the create functions to register are from outgoing_msg respectively incoming_msg.

Regards, Alex
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 13869493
Have to leave now. Would return tomorrow (after 11 hours).

Regards
0
 
LVL 15

Expert Comment

by:efn
ID: 13872840
Given your priorities, you might consider XML (Extensible Markup Language).

http://www.w3schools.com/xml/xml_whatis.asp

It's all text, so you don't have to worry about endianness or structure padding.  It's self-defining, so you can add new fields and a machine that doesn't know about them can still make sense of the rest of the data.  It's human-readable, and there are libraries available to parse and compose it.  There would be some learning required, but it's not terribly abstruse.

Here's a small sample of how it might look:

<header>
  <size>12</size>
  <id>34</id>
  <count>56</count>
  <chksum>102</chksum>
</header>

Obviously, this is not the approach that will send the fewest bytes over the network.

Xerces is well-known library for XML.

http://xml.apache.org/xerces-c/
0
 

Author Comment

by:forums_mp
ID: 13872901

Spent at least an hour here just adding debug statements in an attempt to get a feel for this.  Intrguing - to say the least and certainly gives me an opportunity to learn neat things.  

With reagard to the 'header', one thing that I couldn't quite figure out is.  Where would I update the parameters  
   size;       // Come to think of it, the size here would be the size of buffer.  i.e buffer.size().
   count;     // For each transmission, this gets incremented
   chksum;  // simple addition of t2_data, t3_data and header(size, id, count).  this is somewhat confusing since it would be better to sum the buffer (Yes/No?)

Simply put.  Since the data stream keeps track of the buffer size, this (header stuff) is the sort of stuff I suspect needs to be updated within the datastream.  the id field within the header is fixed (just pick a value - say 0x13) so that's a non-issue.

 bool storeToStream(DataStream& ds)
  {
        static int count;
        msg_header.count = count_;   // for count,  I could do..
        // size and checksum is based on buffer which hasn't been filled yet.. hence?
        ds << msg_header << t2_dat << t3_dat;
        count++;

        return ds.isValid();
  }
0
 
LVL 39

Assisted Solution

by:itsmeandnobodyelse
itsmeandnobodyelse earned 2000 total points
ID: 13873948
If size, count, chksum are parameters of transmission only, you should manage them in DataStream struct, e. g. in getBuffer function.

struct MsgHeader
{
    unsigned short size      :16;
    unsigned short id         :16;
    unsigned short count    :16;
    unsigned short chksum :16;
};

Note, you don't need  :16 if all supported platforms have a 16 bit short (I would say yes).

struct DataStream
{
    string classId;
    string buffer;
    int    curPos;
   
    ...
    int getBufferSize()
   { return isValid()?
        sizeof(MsgHeader)  +     // size + id + count + chksum
        classId.size() + 1 + buffer.size() : 0; }

    int getBuffer(char*& buf, int siz)
    {
        int sizNeeded = getBufferSize();
        int sizClass  = classId.size();
        int sizMsg    = buffer.size();
        if (buf == NULL || siz < sizNeeded)
        {
            delete buf;
            buf = new char[sizNeeded];
        }
        MsgHeader h = { (unsigned short)sizNeeded, getId(), getCount(), getChkSum(buffer.c_str(), sizMsg) };
        if (isBigEndian())
        {
             h.size = ntohs(h.size);
             h.id    = ntohs(h.id);
             h.count = ntohs(h.count);
             h.chksum = ntohs(h.chksum);
        }
        memcpy(buf, &h, sizeof(h));
        memcpy(buf, classId.c_str(), sizClass);
        buf[sizClass] = ':';
        memcpy(&buf[sizClass+1], buffer.c_str(), sizMsg);
        return sizNeeded;
    }
};

You would need to supply functions isBigEndian(), getId(), getCount(), getChkSum() (return is unsigned short) or pass them as arguments to getBuffer.

Regards, Alex
0
 

Author Comment

by:forums_mp
ID: 13875902
Alex, stay tuned.  I'll be testing and I'm sure banging my head in a few :)
0
 

Author Comment

by:forums_mp
ID: 13882400

Curiosity question coupled with your comment "Note, you don't need  :16 if all supported platforms have a 16 bit short (I would say yes).".
Referencing my initial struct (t2_data_t).  I've got 16 bits here where the lower 12 bits singed value.  Going across the link this becomes 2 eight bit values.  How does loadFromStream unwrap the buffer to reflect the initial layout (12 bits signed 4 unsigned)?

struct t2_data_t {
  // later

  signed int       alvaz        :12;
  unsigned int                  : 4;
// more
};
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 13883359
>>>> How does loadFromStream unwrap the buffer
>>>> to reflect the initial layout (12 bits signed 4 unsigned)?

It doesn't. The streaming operations above are *byte* operations (memcpy copies bytes). Also big-endian/little-endian are operating on bytes. Bit stream variables having a bit size different to 8, 16 or 32 must be aligned to byte boundaries *before* moving them to the transmission buffer. So, if you think you couldn't waive on bitstreams in your structs, at least when converting them to the transmission buffer, you *have* to store them into int16, int32, or int64 variables. The bitstreams I used (:16) were used exactly for that purpose.

BTW, if there are differences in size of short/int between the platforms you are supporting, you should define new integer types Int8, Int16, Int32, Int64 - specific for each platform. These types should be used when building the transmission buffers. If you are using struct types for these, you also could provide constructors to handle the endian-issue in a convinient manner.

Regards, Alex
0
 

Author Comment

by:forums_mp
ID: 13887880

Alex, here's an interesting conundrum.  When I compile your source under Visual Studio 7.0 I obtain a couple of warnings that - yeah I sould sort through.

------ Build started: Project: usenet, Configuration: Debug Win32 ------

Compiling...
input_iter.cpp
c:\test\alex\input_iter.cpp(24) : warning C4267: 'initializing' : conversion from 'size_t' to 'int', possible loss of data
c:\test\alex\input_iter.cpp(31) : warning C4018: '<=' : signed/unsigned mismatch
c:\test\alex\input_iter.cpp(34) : warning C4267: 'return' : conversion from 'size_t' to 'int', possible loss of data
c:\test\alex\input_iter.cpp(39) : warning C4267: 'initializing' : conversion from 'size_t' to 'int', possible loss of data
c:\test\alex\input_iter.cpp(84) : warning C4267: '=' : conversion from 'size_t' to 'int', possible loss of data
c:\test\alex\input_iter.cpp(94) : warning C4267: '+=' : conversion from 'size_t' to 'int', possible loss of data
c:\test\alex\input_iter.cpp(95) : warning C4018: '<' : signed/unsigned mismatch
Linking...

//////// NOW

The same code under Visual Studio 6.  results in '86' warnings.   A 'synopsis'.  

Compiling...
main.cpp
C:\Program Files\Microsoft Visual Studio\VC98\INCLUDE\istream(547) : warning C4530: C++ exception handler used, but unwind semantics are not enabled. Specify -GX
C:\Program Files\Microsoft Visual Studio\VC98\INCLUDE\xtree(120) : warning C4786: 'std::_Tree<std::basic_string<char,std::char_traits<char>,std::allocator<char> >,std::pair<std::basic_string<char,std::char_traits<char>,std::allocator<char> > const ,
Persistent * (__cdecl*)(void)>,std::map<std::basic_string<char,std::char_traits<char>,std::allocator<char> >,Persistent * (__cdecl*)(void),std::less<std::basic_string<char,std::char_traits<char>,std::allocator<char> > >,std::allocator<Persistent * (
__cdecl*)(void)> >::_Kfn,std::less<std::basic_string<char,std::char_traits<char>,std::allocator<char> > >,std::allocator<Persistent * (__cdecl*)(void)> >' : identifier was truncated to '255' characters in the debug information
        C:\Program Files\Microsoft Visual Studio\VC98\INCLUDE\map(46) : see reference to class template instantiation 'std::_Tree<std::basic_string<char,std::char_traits<char>,std::allocator<char> >,std::pair<std::basic_string<char,std::char_traits<
char>,std::allocator<char> > const ,Persistent * (__cdecl*)(void)>,std::map<std::basic_string<char,std::char_traits<char>,std::allocator<char> >,Persistent * (__cdecl*)(void),std::less<std::basic_string<char,std::char_traits<char>,std::allocator<cha
r> > >,std::allocator<Persistent * (__cdecl*)(void)> >::_Kfn,std::less<std::basic_string<char,std::char_traits<char>,std::allocator<char> > >,std::allocator<Persistent * (__cdecl*)(void)> >' being compiled
        C:\sw_dev\sp\test02\main.cpp(110) : see reference to class template instantiation 'std::map<std::basic_string<char,std::char_traits<char>,std::allocator<char> >,Persistent * (__cdecl*)(void),std::less<std::basic_string<char,std::char_traits<
char>,std::allocator<char> > >,std::allocator<Persistent * (__cdecl*)(void)> >' being compiled
C:\Program Files\Microsoft Visual Studio\VC98\INCLUDE\xtree(120) : warning C4786: 'std::_Tree<std::basic_string<char,std::char_traits<char>,std::allocator<char> >,std::pair<std::basic_string<char,std::char_traits<char>,std::allocator<char> > const ,
Persistent * (__cdecl*)(void)>,std::map<std::basic_string<char,std::char_traits<char>,std::allocator<char> >,Persistent * (__cdecl*)(void),std::less<std::basic_string<char,std::char_traits<char>,std::allocator<char> > >,std::allocator<Persistent * (
__cdecl*)(void)> >::_Kfn,std::less<std::basic_string<char,std::char_traits<char>,std::allocator<char> > >,std::allocator<Persistent * (__cdecl*)(void)> >::const_iterator' : identifier was truncated to '255' characters in the debug information

The issue I belive is this line.
static map<string, CreateInstanceFunc> factoryMap;

Perfectly legal to me.  I dont get it?
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 13888191
In VC6 you need

#pragma warning ( disable : 4786 )

as very first statement in all cpp files when using stl. That is because a compiler bug.

In VC7 it is a signed/unsigned warning most likely caused by comparisions with sizeof(..) that is unsigned with an 'int' argument (signed). You could/should cast one of the operands to get rid of the warning.

Regards, Alex
0
 

Author Comment

by:forums_mp
ID: 13891231
So I'm putting together a quick test appl.

//////////////  1  //////////////
// In a file called "commrecorder.h"

#ifndef  REC_H
#define REC_H

#include <qobject.h>
#include <qstring.h>
#include "header.h"

#include "data_stream.h"  // NOTE here...

class my_data_class : public persistent  // Use this struct as a start.  if this works.. we're happy
{
  int i;
  double d;
  string  s;
  bool    b;
  char    c;    
public:
  static persistent* create_my_data_class() { return new my_data_class; }
  my_data_class() : i(1), d(2.), s("abc"), b(false), c('X')
  {
    registerClass("my_data_class", create_my_data_class);
  }

  string getClassId() { return "my_data_class"; }
  bool loadFromStream(data_stream& ds)
  {
    ds >> i >> d >> s >> b >> c;
    return ds.isValid();
  }
  bool storeToStream(data_stream& ds)
  {
    ds << i << d << s << b << c;
    return ds.isValid();
  }
};

class QSocket;
class commrecorder: public QObject
{
  Q_OBJECT
public:
  commrecorder();
  ~commrecorder();

///////////////////////
// more stuff
// later
private:
  my_data_class my_data;    // my_data_class instantiated here.
///////////////////////
// more stuff
};

#endif  // end commrecorder.h

//////////////  2  //////////////
Now data_stream.h (included in 1 above) contains all the stuff in your 04/26/2005 03:12AM PDT post.

IOW, in a file called 'data_stream.h'

#ifndef DATA_STREAM_H
#define DATA_STREAM_H

#include <iostream>
#include <string>
#include <map>
using namespace std;
#include <process.h>

struct data_stream
{

 // lots of stuff.

map<string, CreateInstanceFunc> persistent::factoryMap;

#endif  // end data_stream.h

//////////////  3  //////////////
In a third file, called "recorderwin_impl.h" I have

#ifndef RECORDERWIN_IMPL_H
#define RECORDERWIN_IMPL_H

#include "recorderwin.h"

class CommRecorder;   // forward declaration of CommRecorder

class RecorderWinImpl : public RecorderWin
{
  Q_OBJECT
  // later
private:
  CommRecorder  *mComm;
};

#endif


When I attempt to link the application.  I end up with

Linking...
recorderwin_impl.obj : error LNK2005: "struct data_stream & __cdecl operator<<(struct data_stream &,class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > const &)" (??6@YAAAUdata_stream@@AAU0@ABV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@@Z) already defined in commrecorder.obj

recorderwin_impl.obj : error LNK2005: "struct data_stream & __cdecl operator>>(struct data_stream &,class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > &)" (??5@YAAAUdata_stream@@AAU0@AAV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@@Z) already defined in commrecorder.obj

recorderwin_impl.obj : error LNK2005: "private: static class std::map<class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> >,class persistent * (__cdecl*)(void),struct std::less<class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > >,class std::allocator<struct std::pair<class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > const ,class persistent * (__cdecl*)(void)> > > persistent::factoryMap" (?factoryMap@persistent@@0V?$map@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@P6APAVpersistent@@XZU?$less@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@@2@V?$allocator@U?$pair@$$CBV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@P6APAVpersistent@@XZ@std@@@2@@std@@A) already defined in commrecorder.obj

The fact that I include data_stream in (#include "data_stream.h) in commrecorder.h is the problem here.  The last linker error points to this
"map<string, CreateInstanceFunc> persistent::factoryMap;" in data_stream.h

Just when I think I have this stuff figured out (after reading books), what's the solution.?
0
 

Author Comment

by:forums_mp
ID: 13891767

Alex, the first two linker errors are obviously the result of these (currently commented out operator <<, operator >> ) public functions ..

#ifndef DATA_STREAM_H
#define DATA_STREAM_H

#include <iostream>
#include <string>
#include <map>
using namespace std;

#include <process.h>


struct data_stream
{
  string classId;
  string buffer;
  int    curPos;

  data_stream(string clsId)
    : classId(clsId), curPos(0)
  {
  }
   
  data_stream(char* buf, int size)
    : curPos(0)
  {
    string temp(buf, size);
    int pos = temp.find(':');
    if (pos == string::npos || pos == 0 || pos == size-1)
      return;
    classId = temp.substr(0, pos);
    buffer  = temp.substr(pos+1);
  }
   
  bool isValid() { return curPos <= buffer.length(); }
  operator bool() { return isValid(); }
   
  int getBufferSize() { return isValid()? classId.size() + 1 + buffer.size() : 0; }
   
  int getBuffer(char*& buf, int siz)
  {
    int sizNeeded = getBufferSize();
    int sizClass  = classId.size();
    if (buf == NULL || siz < sizNeeded)
    {
      delete buf;
      buf = new char[sizNeeded];
    }
    memcpy(buf, classId.c_str(), sizClass);
    buf[sizClass] = ':';
    memcpy(&buf[sizClass+1], buffer.c_str(), buffer.size());
    return sizNeeded;
  }
};

#if 0   // COMMENTED OUT START
template <class T>
data_stream& operator<<(data_stream& ds, const T& t)
{
  if (ds.isValid())
  {
    int curPos   = ds.curPos;
    ds.curPos += sizeof(T);
    ds.buffer.resize(ds.curPos);
    memcpy(&ds.buffer[curPos], &t, sizeof(t));
  }
  return ds;
}

template <class T>
data_stream& operator>>(data_stream& ds, T& t)
{
  if (ds.isValid())
  {
    int curPos   = ds.curPos;
    ds.curPos += sizeof(t);
    if (ds.isValid())
      memcpy(&t, &ds.buffer[curPos], sizeof(t));
  }
  return ds;
}

data_stream& operator<<(data_stream& ds, const string& s)
{
  if (ds)
  {
    ds.buffer += s;
    ds.buffer += '\0';
    ds.curPos  = ds.buffer.size();
  }
  return ds;
}

data_stream& operator>>(data_stream& ds, string& s)
{
  if (ds)
  {
    s          =  &ds.buffer[ds.curPos];
    ds.curPos  += s.size();
    if (ds.curPos < ds.buffer.length() && ds.buffer[ds.curPos] == '\0')
      ds.curPos++;  // add terminating zero
  }
  return ds;
}
#endif  // COMMENTED OUT END

class persistent;

typedef persistent* (*CreateInstanceFunc)();

class persistent
{
    // factory map is mapping class names to create functions
  static map<string, CreateInstanceFunc> factoryMap;
public:
  virtual string         getClassId() = 0;
  virtual bool           loadFromStream(data_stream& ds) = 0;
  virtual bool           storeToStream(data_stream& ds) = 0;
   
  static persistent* createInstance(const data_stream& ds)
  {
    map<string, CreateInstanceFunc>::iterator it;
    if ((it = factoryMap.find(ds.classId)) == factoryMap.end())
      return NULL;
       
    return it->second();
  }
   
  static bool registerClass(const string& className, CreateInstanceFunc createFunc)
  {
    map<string, CreateInstanceFunc>::iterator it;
    if ((it = factoryMap.find(className)) != factoryMap.end())
      return false;    // already registered
       
    factoryMap[className] = createFunc;
      return true;
  }
   
  data_stream*  store()
  {
    data_stream* pds = new data_stream(getClassId());
    if (!storeToStream(*pds))
    {
      delete pds;
      return NULL;
    }
    return pds;
  }
};

//map<string, CreateInstanceFunc> persistent::factoryMap;

#endif

For the last linker error, I ended up commenting out the line
//map<string, CreateInstanceFunc> persistent::factoryMap;

Not necessarily a good idea, I suspect but ..

Given my current test application, whats the right way to appoach all this, withouth getting linker errors.
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 13892675
>>>> map<string, CreateInstanceFunc> persistent::factoryMap;

static class members may be defined (instantiated) only once. So, you have to move the statement from the header file to one cpp file, e. g. persistent.cpp.

>>>> currently commented out operator <<, operator >> 

Define the operators as 'inline'. That prevents from being instantiated more than once:

template <class T> inline
data_stream& operator<< (data_stream& ds, const T& t)
{
    ....
}  

Regards, Alex


0
 

Author Comment

by:forums_mp
ID: 13900857

Alex
Got busy and sidetracked for a few days but I'm back to coding again :).  Now I'm getting an exception (see below for 'EXCEPTION HERE') here that's driving me crazy.

$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$

#ifndef COMMRECORDER_H
#define COMMRECORDER_H

#include <qobject.h>
#include <qstring.h>
# include <string>
#include "header.h"

# include "data_stream.h"  // Alex's 04/26/2005 03:12AM PDT implementation
# include "persistent.h"      

struct my_data_class : public persistent
{
  int i;
  double d;
      std::string  s;
  bool    b;
  char    c;    

  static bool registered;

  static persistent* create_my_data_class() { return new my_data_class; }
  my_data_class()
      :  i(1), d(2.), s("abc"), b(false), c('X')
  {
    //registerClass("my_data_class", create_my_data_class);    //(2)
  }
  // later
};

class QSocket;
class CommRecorder : public QObject
{
  Q_OBJECT
public:
  CommRecorder();
  ~CommRecorder();
// later
private:
  my_data_class my_data;
};

#endif

$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$

// commrecorder.cpp

#include "commrecorder.h"
#include "data_stream.h"
#include <qsocket.h>

bool my_data_class::registered =
  persistent::registerClass("my_data_class", my_data_class::create_my_data_class); // (1)

CommRecorder::CommRecorder()
{
  // stuff
}

$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$

Here's the registerClass function in persistent.cpp

bool persistent::registerClass(
    const string& className,
            CreateInstanceFunc createFunc
  )
{
  map<string, CreateInstanceFunc>::iterator it;
  if ((it = factoryMap.find(className)) != factoryMap.end())   // EXCEPTION HERE
    return false;    // already registered
       
  factoryMap[className] = createFunc;
    return true;
}

// persistent.h
#ifndef PERSISTENT_H
#define PERSISTENT_H

# include<map>
# include<string>
# include "data_stream.h"

class persistent;
typedef persistent* (*CreateInstanceFunc)();

class persistent
{
  // factory map is mapping class names to create functions
      static std::map<std::string, CreateInstanceFunc> factoryMap;
public:
      virtual std::string    getClassId() = 0;
  virtual bool           loadFromStream(data_stream& ds) = 0;
  virtual bool           storeToStream(data_stream& ds) = 0;
   
  static persistent* createInstance(const data_stream& ds);
   
  static bool registerClass(
             const std::string& className,
          CreateInstanceFunc createFunc
        );
   
  data_stream*  store();
};

The call marked (1) executes and calls the registerClass member function in persistent.cpp.   Except when I step into code with visual studio.   The trouble spot is the spot marked ' _Nodeptr _Pnode = _Root();  ' (see code below).

_Nodeptr _Lbound(const key_type& _Keyval) const
{      // find leftmost node not less than _Keyval
  _Nodeptr _Pnode = _Root();                    
  _Nodeptr _Wherenode = _Myhead;      // end() if search fails
  while (!_Isnil(_Pnode))
    if (this->comp(_Key(_Pnode), _Keyval))
      _Pnode = _Right(_Pnode);      // descend right subtree
    else
      {      // _Pnode not less than _Keyval, remember it
      _Wherenode = _Pnode;
      _Pnode = _Left(_Pnode);      // descend left subtree
     }

    return (_Wherenode);      // return best remembered candidate
}
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 13901011
The problem most likely is because of

>>>> bool my_data_class::registered =
>>>>    persistent::registerClass("my_data_class", my_data_class::create_my_data_class); // (1)

That is an initialization of a static member variable that is done at start of program. Unfortunately, member 'persistent::factoryMap' is a static member as well, but there is no means in C++ to say that persitent::factoryMap should be created *before* my_data_class::registered. It was a 50-50 chance and you lost ;-). You could put both static initialiations to one cpp and create factoryMap before registered. Or you call the registerClass function(s) in the main function explicitly, not using the static member 'registered' (recommended). Or you set my_data_class::registered to false and call the register function in the constructor. The disadvanatge of this is that you have to create at least one instance of my_data_class *before* you could use the *factory* to create instances from the communication.

Regards, Alex
0
 

Author Comment

by:forums_mp
ID: 13901182

OK, I think I'll go back to your initial approach.  i.e let my_data_class handle 'it's' registeration the way you had it in your INITIAL post), and get rid of the static member registered.

  my_data_class()
   :  i(1), d(2.), s("abc"), b(false), c('X')
  {
    registerClass("my_data_class", create_my_data_class);   /// here
  }

////////////// 2   The sendMessage member function below is called at a 10 Hertz rate.   So now:

void CommRecorder::sendMessage()
{
  if(!mConnected)
    return;

  my_data.i = 5;
  my_data.d = 5;
  my_data.s = "test";
  my_data.b = true;
  my_data.c = 5;

  data_stream* pds = my_data.store();
  char *buf(0);
  int  size(0);
  if ((size = pds->getBuffer(buf, 0)) > 0)
  {
    // send buffer
    mSocket->writeBlock(buf, size);                 // (1)  
    data_stream  ds(buf, size);                        // (2)
// AFTER    mSocket->writeBlock(buf, size);    // (3)

    if (ds)
    {
      persistent* pNew = persistent::createInstance(ds);
      if (pNew)
        cout << "Created new instance of class " << pNew->getClassId() << endl;
      else
        cout << "Creation failed" << endl;
     }
  }
  else
  {
    // report some kind of failure
  }
}

A few questions to ensure I fully understand this (before I add in real messages and get very dangerous :))
  The getBuffer function returns (via the argument char*&buf) a buffer with a classId + message

The intanantiation of     " data_stream  ds(buf, size); " .. line marked (2) above is done after the call to the QT member function writeBlock.   With this approach (line marked 1) before (line marked 2), I'll end up sending the classId + message.  I dont believe I want to send the classId with the message.   Put another way.  Does teh receiver care about the classId that the sender sent the message from?

Trouble is it also doesn't make sense to instantiate the ds before the call to writeBlock, since ds is only used within the createInstance member function of persistent.

In some respects, I'm confused on item (2).  Why the need to update the object buffer (see below) and classId in data_stream when they've already been updated by getBuffer?

  data_stream(char* buf, int size)
    : curPos(0)
  {
    string temp(buf, size);
    int pos = temp.find(':');
    if (pos == string::npos || pos == 0 || pos == size-1)
      return;
    classId = temp.substr(0, pos);
    buffer  = temp.substr(pos+1);
  }

The answer might be that I need to create the call to createInstance, which begs another.  Every 10 Hertz, I'll call createInstance - as its currently shown above?
0
 
LVL 39

Assisted Solution

by:itsmeandnobodyelse
itsmeandnobodyelse earned 2000 total points
ID: 13902322
>>>> OK, I think I'll go back to your initial approach.

That's good for the sender's site. On the receiver's site you need to register factory *before* you  try to use it. So, you need to register before or while you are initialzing the communication.

>>>> Does teh receiver care about the classId that the sender sent the message from?

It should. It could dynamically create an instance of th eclass on the receiver side b y using a registered create function that could be found by the class id. Of course, if you *know* what kind of message you get, you simply could provide a constructor that takes a message buffer. The factory is needed if there are at least two different kind of messages and if you want an automatism ( a very clever and *magic* automatism).

>>>> I'm confused on item (2).  Why the need to update the object buffer (see below) and classId in
>>>> data_stream when they've already been updated by getBuffer?

Of course you don't need both  at the sender's site.

The sender:

- creates an empty datastream objet
- uses streaming operators to fill the transmission buffer (in storeToStream)
- get the streaming buffer and size
- writes buffer to communication


The receiver:

- reads buffer + size
- creates a datastream object by passing buffer + size
- uses factory to create an object of a calss equivalent to the my_data_class
  object of the sender. Note, it might be a different class, only class id must be
  identically.


In my sample above I added sender + receiver in one code snippet, just to demonstrate the possibilities.

Regards, Alex

0
 

Author Comment

by:forums_mp
ID: 13903159

>>> That's good for the sender's site. On the receiver's site you need to register factory >>>*before* you  try to use it. So, you need to register before or while you are
>>> initialzing the communication.
I see.  Your sender/receiver illustration above certainyl cleared things up.  Now I'm following the importance of the 'factory'.

If only I understood this as well as you do.   I konw its all 'relative' but playing with fourier transforms (DSP) is a lot easier than all this :)

In any event ... I'll have a few more questions .. so :)

0
 

Author Comment

by:forums_mp
ID: 13919498

I pulled out one of my C++ text last night but got side tracked.   I'll go back to one of your earlier response this linker issue

//map<string, CreateInstanceFunc> persistent::factoryMap;
>>>static class members may be defined (instantiated) only once. So, you have to move the statement from the header file to one cpp file, e. >>>g. persistent.cpp.

Understood, so lets move the statement (1) below to persistent.cpp,  So now I have:

# include "persistent.h"
# include "data_stream.h"

map<string, CreateInstanceFunc> persistent::factoryMap;      /// (1)
// createInstance etc. etc.
///


// in persistent.h
#ifndef PERSISTENT_H
#define PERSISTENT_H

struct data_stream;  // persistent methods needs data_stream so forward declare dont include.
class persistent;
typedef persistent* (*CreateInstanceFunc)();

class persistent {
  //
}

#endif

In a header file called 'data_stream.h"

#ifndef DATA_STREAM_H
#define DATA_STREAM_H

#include <iostream>
#include <string>
#include <map>
using namespace std;

struct data_stream
{
  string classId;
  string buffer;
  int    curPos;

  data_stream(string clsId)
    : classId(clsId), curPos(0)
  {
  }
 // all the data_stream stuff
};

The linker complains that data_stream has already include factoryMap.
0
 

Author Comment

by:forums_mp
ID: 13919728
Alex,  I think I figured it out.
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 13920256
>>>> Alex,  I think I figured it out.

Good, the instantiation of factoryMap has to happen only in one cpp file. You have to move it from header file if the header was included by two or more cpp files.

Regards, Alex
0
 

Author Comment

by:forums_mp
ID: 13920442

Alex, so I thought before I get carried away, I'd perform some basic validation.   persistent and data stream is working :)

on the receive side i have code akin to;

   const size_t RCVBUFFER_SIZE (64);
   char buffer[RCVBUFFER_SIZE];
  //  later
  while (  rcv_msg_size = sock->receive (buffer, RCVBUFFER_SIZE ) )
  {
     cout << rcv_msg_size << endl;

     for (int idx(0); idx < rcv_msg_size; ++idx )
       cout <<  buffer[idx];
     cout << endl;
  }

When the sender transmits the 'receive' member function wakes up.  Trouble is the cout prints
   my_data_class: |
Where " | " is some funny character.

The contents of my_data_class is outlined below.  Is there a way to dump a char buffer in a meanigful context?  

struct my_data_class : public persistent
{
  int i;
  double d;
  std::string  s;
  bool    b;
  char    c;    
 
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 13924147
>>>> cout <<  buffer[idx];

outputing binary data isn't that simple. You could do it like a hex editor:

  0D0A 0032 3232 0000 00BA 3031 4142 4344     . . . 2 2 2 . . . . . A B C D

The leftside prints hex words (16 bits), the right part prints printable characters.

Or maybe better: You first do the conversion via factory and use operator<< of the resulting class to make the printout.

Regards, Alex

0
 

Author Comment

by:forums_mp
ID: 13926967

Alex.... Consider my approach on the receive side.

void some_function()
{
  while (( rvcdMsgSize = socket->receive(echo_buff, RCVBUFSIZE)) > 0)  
  {
    cout << rvcdMsgSize << endl;                            // (1)

    data_stream stream(echo_buff, rvcdMsgSize);     // (2)

    if (stream)
    {
      my_data_class *ptr_msg =
          dynamic_cast<my_data_class*>(persistent::createInstance(stream));
      if (ptr_msg)
      {
        if (ptr_msg->loadFromStream(stream))          // (3)
        {
          cout << " i " << ptr_msg->i << endl;
          cout << " d " << ptr_msg->d << endl;
          cout << " s " << ptr_msg->s << endl;
          cout << " b " << ptr_msg->b << endl;
          cout << " c " << ptr_msg->c << endl;
        } else { cout << " loadFromStream returned false " << endl; }
      }
      else { cout << " returning false " << endl; }
    }
   // more
  }
  delete socket;

}

The call to loadFromStream fails 'everytime'.  So after massive couts :) I discovered that the impetus behind the failure is as follows:  First  here's what the sender sends:

  my_data.i = 99;
  my_data.d = 9;
  my_data.s = "testing";
  my_data.b = true;
  my_data.c = 'A';

  data_stream* pds = my_data.store();
  char *buf(0);
  int  size(0);
  if ((size = pds->getBuffer(buf, 0)) > 0)
  {
    // send buffer
    mSocket->writeBlock(buf, size);
  }

>>>>>>>> On the receive side, I receive 36 bytes.  i.e the cout  (item 1) above:
Prints 36.  
This - of course matched what the send 'sent'.  So far so good.

When I create instantiate data stream above (item 2 above).
buffer.length() equals     - 22
buffer.size()    equals      - 22
classId.size()   equals     - 13

22 + 13 = 35.   That said, a character gets dropped but I'll look at 'who' in a minute.

Item 3, is where things gets interesting.  With regard to loadFromStream, here's the breakdown:

1)
   ds >> i ;
curPos = 4.  
cout << i <<  endl;   = 160944384;
Recall I sent 99.  Endian issues... (I suspect, will investigate in a few).

2) ds >> d;
curPos = 12;
cout << d <<  endl;   = 4.33197e-320;
I sent 9. Same story

3) ds >> s;
curPos = 20;
cout << s << endl;   =  "testing".  This came out OK.

4) ds >> b;
curPos = 24;    // SEE LINE ( CC)  below


template <class T>
inline data_stream& operator>>(data_stream& ds, T& t)
{
  if (ds.isValid())                                       // (AA)
  {
    int curPos   = ds.curPos;
    ds.curPos += sizeof(t);                         // (BB)
    if (ds.isValid())                                     // (CC)
    {
      memcpy(&t, &ds.buffer[curPos], sizeof(t));
    }
  }
  return ds;
}

As a result (CC) never executes and rightly so.  The length of the buffer is 22 bytes but now were up to 24 bytes.

cout << b << endl;   =  0.  
The output here is the default initilization of b.  Make sense since there's failure above.

The last item

5) ds >> c;
curPos = 24;    // Remains at 24 cause (AA) above failed
cout << c << endl;   =  S.  
The output here is the default initilization of c.  Make sense since there's failure above.


//////////
sizeof on both platforms...
.NET 2003                                           GCC
-----------------------------------|----------------------------------
sizeof(int)              =      4                    4
sizeof (std::string)  =    28                    4
sizeof (double)       =      8                    8
sizeof (float)          =      4                    4
sizeof (char)          =      1                    1
sizeof (bool)          =      1                    4

Recommendations on a solution to this?




0
 

Author Comment

by:forums_mp
ID: 13927426

Alex, one other thing I forgot to type.  This amounts to a difference in type sizes... In this case when I load the 'struct' with the contents of the buffer, alls well until I encounter the bool value.
On GCC a bool is 4 bytes, while on .NET it's 1.

This shoots currPos pass the buffer length and as a result my troubles.

I thought using a buffer of bytres was supposed to take care of this :)
Another potential problem I see is the size of string .. On .NET its 28 bytes, on GCC it's 4.   Potential problem for a really long string?   The quetion then becomes.  How do I get curPos to coorperate with the buffer ..  ie platform independent...   Does that even make sense?

0
 
LVL 39

Assisted Solution

by:itsmeandnobodyelse
itsmeandnobodyelse earned 2000 total points
ID: 13928290
>>>> Recommendations on a solution to this?

std::string doesn't matter as you couldn't transfer a string object as it is cause it has (an) internal pointer member(s). So, you need to transfer std::string to a zero-terminated or fix-sized char array and vice versa. I already gave you a valid implementation of these operators (see above).

bool type is more serious. Maybe it is an option to not using them or you need operator<< and operator>> that turn a bool to an unsigned char and back:

  datastream& operator<< (datastream& ds, bool b)
  {
         unsigned char uc = b? 0x01 : 0x00;
         ds << uc;    // uses template operator<<
         return ds;
  }  

  datastream& operator>> (datastream& ds, bool& b)
  {
         unsigned char uc;
         ds >> uc;    // uses template operator>>
         b = (uc)? true : false;
         return ds;
  }

>>>> How do I get curPos to coorperate with the buffer

Using the operator functions above, curPos is incremented by 1 (size of unsigned char) at all platforms.


Regards, Alex
0
 

Author Comment

by:forums_mp
ID: 13928518
Impressive.  

What woudl you recommend for the case where - lets say - .NET was 32 and GCC - 16 bit ints?

What's the story on floats and doubles.   Are those always the same like char or ??...
Basically what I'm after is an option to keep 'all that in my rear pocket' should I need to use it.

 Granted I should meet the _needs_ of my end user, but I'll be discussing with the  end user the idea that we should rid the bit fields.
I recall you stating that bit fields will make things slightly more complex (not necessarily in those words) but I'd like to get my head around what this means to current design.

Could you inform me about potential pitfalls with current design (if any) with respect to my inital (first post) struct that contained bit fields?
0
 
LVL 39

Assisted Solution

by:itsmeandnobodyelse
itsmeandnobodyelse earned 2000 total points
ID: 13929998
>>>> ... for the case where - lets say - .NET was 32 and GCC - 16 bit ints?

You would need to have operators for int type where with GCC all 16 bit ints go to a 32 bit int and back

 datastream& operator<< (datastream& ds, int i)
 {
       union IntToUchar
       {
            unsigned char  uc4[4];   // 4 * 8 bit
            int                  i2[2];      // 2 * 16 (GCC)  or  2*32 (else)
       };

       IntToUchar Int;
       Int.i2[0] = i;    // first integer is i
       Int.i2[1] = (i >= 0)? 0 : -1;   // second int is 0 or -1 (all bits set)
       for (int n = 0; n < 4; ++n)
            ds << Int.uc4[n];    // uses template operator<<
       return ds;
 }  

>>>> What's the story on floats and doubles

You need to check if float and doubles have same representation on all platforms. If not, best would be to convert them to string decimals, e. g. by using stringstream, and store them as strings. Same you could do with all other numeric types.

With bit fields you could make your integers all 16 bit, but you should know if you do want that. Bit fields can be defined in structs only, so you would need to care about overflow when using any temporary integer. IMHO, bit fields shouldn't be used in normal structs/classes but only for bit conversions.

Regards, Alex
0
 

Author Comment

by:forums_mp
ID: 13930728

One other question for you.  At the beginning of my "05/04/2005 07:25AM PDT" post the code resident with some_function reflects what's running on my GCC platform.   So I added an 'echo' option.  IOW.  I'll xmit info received.

void some_function()
{

  // more
  // additions now.
    data_stream* pds = ptr_msg->store();
    char *buf(0);
    int  size(0);
    if ((size = pds->getBuffer(buf, 0)) > 0)
    {
    // send buffer
      if ( sock->send(buf, size) )
        cout << " send success " << endl;
    }
    else
    {
      // report some kind of failure
    }
  }
  cout << recv_msg_size << endl;  
  delete sock;
}

On the PC side. I've got:

void CommRecorder::slotRead()
{
  int recv_msg_size = mSocket->readBlock(mBuffer, 10000);

  data_stream stream(mBuffer, recv_msg_size);

  if (stream)
  {
    my_data_class *ptr_msg =
        dynamic_cast<my_data_class*>(persistent::createInstance(stream));    // (99)
    if (ptr_msg)
    {
      if (ptr_msg->loadFromStream(stream))
      {
      } else {

        }
    }
    else {
      //cout << " returning false " << endl;
      }
  }

Microsoft complains.  As a result I get an exception:
  Unhandled exception @ 0x77E75887 in app.exe  Microsoft C++ exception:
  __non_rtti_object@0x0012e620.

 I'll have to look up the 'dynamic_cast' more specifically what happens if the cast fails but this works well under GCC, hence I dont get it.  

/////////// 2

Given two structs.  PC_GCC is what the GCC platform receives from teh PC, likewise GCC_PC is what's transmitted from GCC platform to PC.

For clarification, the basic premise surrounds having these two structs in a header (or split across header and source) file on each platform?  So now:

struct PC_GCC {
   unsigned int   val1;
   unsigned int   val2;
  // stuff...

  bool loadFromStream(data_stream& ds)
  {
    ds >> val1  >> val2;
    return ds.isValid();
  }
  bool storeToStream(data_stream& ds)
  {
    ds << val1 << val2;
    return ds.isValid();
  }
};

struct GCC_PC {
  std::string    str1;
  unsinged int   val;
  // stuff
  bool loadFromStream(data_stream& ds)
  {
    ds >> str1 >> val;
    return ds.isValid();
  }
  bool storeToStream(data_stream& ds)
  {
    ds << str1 << val;
    return ds.isValid();
  }
};


Except now I have to register two classes.. No big deal.  
Ultimately, I'll finish this off with MySenderClass and MyReceiverClass you alluded to in an earlier post.

Really cool designs.  I've learned a lot :)  

0
 

Author Comment

by:forums_mp
ID: 13932363

Alex, in addition to my previous post, I've got a few more questions here.  First I need to resort back to a previous post of yours, more specifically your 04/26/2005 05:25AM PDT post.  

[quote]
Endianess must/could be resolved by providing specific streaming operators for all integer types that have to call htons/htonl/ntohs/ntohl before or after streaming operation, e. g. by that:

struct Int32BigEnd { int i; Int32BigEnd(int in) : i(htonl(in)){} };

// on MAC
DataStream& operator<<(DataStream& ds, int i) {
   ds << Int32BigEnd(i);   // use template with big-endian struct type
   return ds;
}
[/quote]

Is it possible to determine the type id within the insertion and extraction operations of the templated member functions.  IOW
template <class T>
inline data_stream& operator >> (data_stream& ds, T& t)
{
   if (type_id == int)  or float or double )
   // do endian conversion..
}
Then again is it safe to state that the insertion and extraction operators of the templated versions cover only int, float double?  If yes, then it might be safe to just do the conversion after all.

**************************************
(2)  The getBuffer function allocates memory on the free store.  How would I go about clean up of the object 'buf'.  i.e
         if (buf == NULL || siz < sizNeeded)
         {
            delete buf;
            buf = new char[sizNeeded];
         }
 Once buf is allocated how does one clean up at completion/termination?


**************************************
(3)  inline data_stream& operator >> (data_stream& ds, bool& b)
      {
        if (ds.isValid())                        // (AA)
        {
           unsigned char uc;
           ds >> uc;
           b == (uc) ? true : false;        //  (CC)
        }
        return ds;
     }

With respect to line CC.

Two things:  In your post you had b = (uc)...  Did you mean b == (uc) ..?

b.  Line CC produce the warning:
warning C4805: '==' : unsafe mix of type 'bool' and type 'unsigned char' in operation

With respect to line AA.  For starters is line AA necessary.  I suspect it doesn.  I also suspect that I could use the version that calls operator bool()
 ie.  I could do
       if (ds)

I noticed you used said version with the string types and I'm just curious as to when to use what?  It appears that either version will suffice.
0
 

Author Comment

by:forums_mp
ID: 13934471

Alex, I just realized something.  This issue (two previous post) where Microsoft complains about __non_rtti_object is simply becasue the stream object is not a pointer.  Oddly enough GCC let it slide.


  data_stream stream(mBuffer, recv_msg_size);

  if (stream)
  {
    my_data_class *ptr_msg =
        dynamic_cast<my_data_class*>(persistent::createInstance(stream));    // (99)
    if (ptr_msg)
    {
      if (ptr_msg->loadFromStream(stream))
      {
      } else {

       }
    }
    else {
      //cout << " returning false " << endl;
     }
  }

Is there a way to avoid '99', more specifically the dynamic_cast?
0
 

Author Comment

by:forums_mp
ID: 13934972
Alex, I suspect I'll sit back and wait for you to catch up but have a look at this dynamic_cast issue (see previous three posts) for me.  if I change to:

   data_stream* stream = new data_stream(mBuffer, recv_msg_size);

Stuff breaks...  For instance.  loadFromStream etc.

0
 
LVL 39

Assisted Solution

by:itsmeandnobodyelse
itsmeandnobodyelse earned 2000 total points
ID: 13935498
>>>> __non_rtti_object@0x0012e620.

You have to debug into the exception. _non_rtti means 'no RunTtme-Type-Information' what could be caused by dynamically casting a NULL pointer returned by createInstance. That typically would happen if you didn't register the create function of my_data_class prior to calling createInstance.

>>>> 2 ///////
>>>> Given two structs.  PC_GCC and GCC_PC

Maybe I didn't unterstand exactly what you are trying to do ... Some comments:

(A)
On the sender site you only need to store. On the receiver site you need to load. But you don't need both on any site if you never send the message (struct) back.

(B)
The receiver may create a different class than the sender was sending by providing an appropriate create function, but two things can *not* be changed: the class ID that identifies the message (and the function to create the class instance) *and* the fact that you operate on the same data record. I. e. if you put an int(eger) to the stream you have to read/extract an 'int' (or 4 bytes or 2 shorts)   at the receiver's site. You couldn't read a string variable if you sent an int. Of course by coding appropriate operator<< functions you may convert an int to a std::string. But that make the things complex where there is no need. It reminds on overiding operator+ to  make subtraction instead of addition.

The concept of factories and serialisation is that - beside of platform differences - both the sender's class and the receiver's class have the same data members they want to share/transfer (Note both may have transient data members that only exist on one site). The easiest way to do that is to have a common baseclass where the sources are identical at both sides. The baseclass could implement both storeToStream and loadToStream - though only one of these is used at one site - and is made portable. With the baseclass operator<< >> functions were provided for all common types that you want to support for serialization. Note, it's good practice to only supporting a few types or converting all to a portable type. Actually, I once made a project that uses string types only and XML transfer is a good sample that other's had the same idea...

>>>> (1) Is it possible to determine the type id

Actually, I don't know (and never heard of). Maybe some compiler's do provide RTTI functions where you could find out template types at runtime, though I would assume that it doesn't work for C types or only in Debug mode...

Anyway, the if statement is very bad and an offense against the idea of templates and polymorphism. You easily could provide a specialization of operator functions where you are 'catching' the type requested. What is more annoying that there is no easy way have the same source on any platform beside of ugly multi-platform coding like the following:

#ifdef WIN32
    doWin();
#elif defined (LINUX)
    doX();
#elif defined (MAC)
    doMac()
#endif

Maybe the htons/htonl/ntohs/ntohl functions give a way out, if they would do nothing on a MAC (already has Network Order). Then, you generally could stream integers to network order and back, automatically getting the required endianess... The disadvantage of that is that you couldn't easily check  your transfer buffer on a little-endianess system cause all integers look weird... So maybe - providing appropriate streaming functions solely on the MAC might be the less complex solution.

>>>> (2)
You need to delete the buffer you got after use. You also could store the buffer in datastream class and delete it in the destructor. I didn't made that cause getBuffer would look like that:

   const char* datastream::getBuffer(int& size);

Here one return value is given by return type and the other by reference, what is kind of a inproper interface. But it could be 'cleaned' by

struct transferbuffer
{
private:
     int       siz;
     char*  buf;
public:
     transferbuffer() : siz(0), buf(NULL) {}
     transferbuffer(int s) : siz(s) { buf = new char[siz];  memset(buf, 0, siz); }
     ~transferbuffer()  { delete []buf; }
     int size() { return siz; }
     const  char* buffer() { return buffer; }
private:
     char* writeableBuffer(int neededSiz)
     {
          if (siz < neededSiz)
          {
               char* p = new char[neededSiz];
               memset(p, 0, neededSiz);
               if (siz > 0)
                   memcpy(p,  buf,  siz);
               delete [] buf;
               buf = p;
               siz = neededSiz;
          }
          return buf;
     }
     friend class datastream;  // may call writeableBuffer()
};

bool datastream::getBuffer(transferbuffer& buf);

      ....

      transferbuffer buf;      
      if (datastream.getBuffer(buf))
      {
              send(sock, buf.buffer(), buf.size(), ...);
              ...
      }  

>>>> (3)
In your post you had b = (uc)...  Did you mean b == (uc) ..?

No, I used the "?:" operator:

     b = (uc)? true : false;

is equivalent to

     if (uc)
          b = true;
     else
          b = false;

or formal:

     <conditional_expression> ?  <value_expression> : <value_expression>

You could use that operator even in the argument list of a function:

      char* buf = getBuffer( (size > minsize)? size : minsize); // take the maximum

>>>> warning C4805: '==' : unsafe mix of type 'bool' and type 'unsigned char' in operation

   (uc)? tests an unsigned char on being not equal to '\0'.

Correctly it would be

    b = (uc != '\0')? : true : false;

Now, (uc != '\0') is a conditional expression of type bool. (uc) does the same (and before type 'bool' became C++ standard type, any bool was mapped to an int; that's why GCC has a 4-byte bool type) but wasn't quite correct. I used it in the sample cause I normally avoid using negative conditions but the positive condition would have been  (uc == 0x01), what is strange (and could give warnings as well).

>>>>     if (ds)

That works cause I added  operator bool() to class datastream. The isValid function is equivalent (and may be more transparent) but not as handy. In C often functions returning pointer where used like that :

    XYZ* func();

    if (func())
        doSomething();

That is bad style and when I first saw a statement like

    if (cin>>x)

I was puzzled cause I wondered why operator>> was returning a pointer. However, I learned that operator<< returns a reference to iostream and that one could test a class object on true or false by simply providing an operator bool() member fucntion. So, sometimes I make the same.

Regards, Alex
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 13935697
>>>> Is there a way to avoid '99', more specifically the dynamic_cast?

I hope my answers in the previous post (it took my some hours cause I was at lunch between) would solve the problem.

Indeed, you wouldn't need a cast of if you would call a virtual function - first defined in class persistent - that would invoke a member function of class my_data_class. In my framework, persistent has a virtual function named 'execute' that was called after createInstance:

   persistent* p = createInstance(stream);
   if (p != NULL)
   {
         p->execute();   // that function does the job
         delete p;
   }

You see, I don't need casting as the 'this' pointer in function execute has the correct class type.

Did you register create function of class my_data_stream prior to calling createInstance?

If you still have problems, break the project  down to a few source files (probably one) where the problem still happens and post it here. I would only need the receiver site and size and (hex) contents of the received buffer.

Regards, Alex
0
 

Author Comment

by:forums_mp
ID: 13937283

>>>> You see, I don't need casting as the 'this' pointer in function execute has the correct class type.

I think this goes back to your 04/26/2005 11:26AM PDT post where you alluded to:

[alex]
BTW, if you would make a baseclass named transmission_msg and derived classes outgoing_msg and incoming_msg, you could make different implementations of virtual functions, say overrides of Persistent::execute(), on the sender site and on the receiver site. The registered class would be 'transmission_msg' but the create functions to register are from outgoing_msg respectively incoming_msg.
[/alex]

I was saving this implementation for 'last'.  I suspect the ideal thing to do is have you provide an example utilizing this approach using my_data_class that I've been playing with.

Here's what I envision from above.  
class data_stream {

};

class persistent {
public:
  // add an execute member fucntion
   virtual void execute() = 0;
};

 class transmission_msg {
  public:
    void execute () {
   }
 };

  class outgoing_msg : public transmission_msg {
  };

  class incoming_msg : public transmission_msg {
  };

How do you roll all this up and tie this to data_stream & persistent.  I think I see how but I'm not so sure..

Thanks
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 13940336
>>>> How do you roll all this up and tie this to data_stream & persistent.  

datastream is just a helper like iostream. It provides the streaming functionality to move the *persistent* data members to a buffer and back. You also could move the functionality of datastream to another class, e. g. persistent, but I wouldn't do that.

persistent is the baseclass the factory could work on. So, all classes that need to be serialized *must*  be derived from persistent as the factory has a persistent pointer as result. So class transmission_msg necessarily need to be derived from persistent.

class transmission_msg : public persistent
{
  public:
    void execute () {               // Note, transmission_msg doesn't need an execute function
   }                                      // but class incoming_msg
 };


By providing a virtual function like execute() the derived classes can get control after creation by the factory. That means a factory is similar to a *virtual constructor* where you simply read an id  from some external source (here your socket) and was able to create an object of any arbitrary class that has registered to the factory.

Regards, Alex


0
 

Author Comment

by:forums_mp
ID: 13941611

Alex, you have to bear with me here....

"So class transmission_msg necessarily need to be derived from persistent.".    For clarification, I think what you're saying is transmission_msg needs to be derived from persistent.

Now I'm missing something trying to implemetn this change.  Here's what I have


#ifndef PERSISTENT_H
#define PERSISTENT_H

# include<map>
# include<string>
//# include "data_stream.h"  /// data_stream hasn't changed

class persistent;
typedef persistent* (*CreateInstanceFunc)();
struct data_stream;

class persistent
{
  // factory map is mapping class names to create functions
  static std::map<std::string, CreateInstanceFunc> factoryMap;
public:
  virtual std::string    getClassId() = 0;
  virtual bool           loadFromStream(data_stream& ds) = 0;
  virtual bool           storeToStream(data_stream& ds) = 0;
  virtual void           execute() = 0;
 
  static persistent* createInstance(const data_stream& ds);
   
  static bool registerClass(
         const std::string& className,
          CreateInstanceFunc createFunc
        );
   
  data_stream*  store();
};

#endif

// msg_transmission .. the .cpp  for msg_transmission is bacially a constructor and destructor
#ifndef MSG_TRANSMISSION_H
#define MSG_TRANSMISSION_H

# include<map>
# include<string>
# include "persistent.h"

class msg_transmission
  : public persistent
{

public:
  msg_transmission();
  ~msg_transmission();
};

class incoming_msg
  : public msg_transmission
{
public:
  incoming_msg();
  ~incoming_msg();
};

////
class outgoing_msg
  : public msg_transmission
{
public:
  outgoing_msg();
  ~outgoing_msg();
};

#endif


// comm_rec.h
#ifndef COMM_REC_H
#define COMM_REC_H

#include <qobject.h>
#include <qstring.h>
# include <string>

# include "data_stream.h"
# include "msg_transmission.h"

struct my_data_class
  : public outgoing_msg
{
  int          i;
  double       d;
  std::string  s;
  bool         b;
  char         c;    

  //static bool registered;
  static persistent* create_my_data_class() { return new my_data_class; }
  my_data_class()
    :  i(1), d(2.), s("abc"), b(false), c('Z')
  {
  }

  std::string getClassId() {
    return "my_data_class";
  }
  bool loadFromStream(data_stream& ds) {
    ds >> i >> d >> s >> b >> c;
    return ds.isValid();
  }

  bool storeToStream(data_stream& ds) {
    ds << i << d << s << b << c;
    return ds.isValid();
  }
};

So comm_rec.h is derived off out_going_msg.  Is this right or am I'm way off on that?  Can you fix all this up for me.. Put it back to where it compiled + with all the requisite changes.  :).    I'm getting all turned around here, trying to implement a fix.
0
 
LVL 39

Assisted Solution

by:itsmeandnobodyelse
itsmeandnobodyelse earned 2000 total points
ID: 13943439
>>>> Is this right or am I'm way off on that?  

What's the purpose of msg_transmission, outgoing_msg and incoming_msg? They haven't any functionality yet. The  only class you could make instances of is my_data_class.

If you want to differ between sender and receiver you would have to derive different classes *from* my_data_class and not from an abstract baseclass.

                          persistent
                                 |
                       my_data_class
                  /                            \
 my_outgoing_data_class    my_incoming_data_class


my_outgoing_data_class is available at the sender site, my_incoming_class at the receiver site. The most derived classes implement getClassId() and nothing else. All other virtual functions were implemented by m_data_class, especially getClassId()- Only  my_incoming_data_class needs to register in the model above cause only my_incoming_data_class objects will be created after receiving a datastream buffer.

Note, there might be different designs where both sites could *receive* a datastream buffer, then both sites may provide a different derived class based on my_data_class. However, such a design wouldn't differ between 'outgoing' and 'incoming' but on function, e. g. my_server_data_class and my_client_data_class.

In Client-Server architecures you need to exchange data objects between the server and it's clients. Here objects may go to both directions, e. g. a client creates a new customer at a front-end and sends it to the server. The server stores the object to a database and sends it to all other active clients in order to make them up-to-date. You see, all object creation in the factory happens by 'anonymous' persistent objects - the server doesn't know what objects were created by the facory - but simply calls the 'execute' function of the newly generated object. Moreover, the server's execute function needs a different implementation than that of the clients, and there might be needs to differ at the client's site between 'outgoing' and 'incoming' data. That all could be done by deriving and registering new classes from the common data baseclass that contains all data members (and nothing else).

Regards, Alex


Regards, Alex

0
 

Author Comment

by:forums_mp
ID: 13945628

I guess that's where I'm getting confused.  In your 05/05/2005 02:21PM PDT post.  Is 'transmssion_msg'  the same as my_data_class in your latest post?  

OK!!   I'm following you now (I think).  So I'll start over.

// comm_rec.h
#ifndef COMM_REC_H
#define COMM_REC_H

# include <string>
 
# include "transmission.h"

class comm_rec :
{
public:
  comm_rec();
  ~comm_rec();

private:
  incoming_message inc_;              //////// defined in transmission.h !!
  outgoing_message out_;              //////// defined in transmission.h !!
};
#endif

/// comm_rec.cpp
#include "commrecorder.h"

comm_rec::comm_rec()
{
}
comm_rec::~comm_rec()
{
}

void comm_rec::xmit()
{
  out_.i = 99;                            /// Now use the outgoing msg  stuff
  out_.d = 9;
  out_.s = "testing";
  out_.b = true;
  out_.c = 'A';

   // finish the store and execute portion.. fill in holes here!!

  // buf has the contents and we know the size so now send the data..
  //msocket->WriteBlock (buf, size);
}

void comm_rec::receive()
{
  // int recv_msg_size = mSocket->readBlock(mBuffer, sizeof(mBuffer));
  /// he'll use the incoming msg stuff.. fill in hole here
}


// transmission.h

#ifndef MSG_TRANSMISSION_H
#define MSG_TRANSMISSION_H

# include<map>
# include<string>
# include "persistent.h"
# include "data_stream.h"

struct rec_incoming_data   // incoming message to comm_rec
   : public persistent
{
  int          i;
  double       d;
  std::string  s;
  std::string  s1;
  std::string  s2;
  bool         b;
  char         c;    

  //static bool registered;
  static persistent* rec_incoming_data_class() { return new rec_incoming_data; }
  rec_incoming_data()
    :  i(1), d(2.), s("abc"), s1("def")
       , s2("ghi"), b(false), c('Z')
  {
  }

  std::string getClassId() {
    return "rec_incoming_data";
  }
  bool loadFromStream(data_stream& ds) {
    ds >> i >> d >> s >> s1 >> s2 >> b >> c;
    return ds.isValid();
  }

  bool storeToStream(data_stream& ds) {
    ds << i << d << s << s1 << s2 << b << c;
    return ds.isValid();
  }

  void execute()
  {
      // What happens here ?????
  }

};


struct rec_outgoing_data   // outgoing_msg FROM comm_rec
  : public persistent
{
  int          i;
  double       d;
  std::string  s;
  bool         b;
  char         c;    

  static persistent* rec_outgoing_data_class() { return new rec_outgoing_data; }
  rec_outgoing_data()
    :  i(1), d(2.), s("abc"), b(false), c('Z')
  {
  }

  std::string getClassId() {
    return "rec_outgoing_data";
  }
  bool loadFromStream(data_stream& ds) {
    ds >> i >> d >> s >> b >> c;
    return ds.isValid();
  }

  bool storeToStream(data_stream& ds) {
    ds << i << d << s << b << c;
    return ds.isValid();
  }

  void execute()
  {
    // REALLY NOT NEEDED BUT
   // HAVE TO HAVE SINCE WE DERIVE OFF PERSISTENT
  }

};

// now we're ready
class incoming_message
  : public rec_incoming_data
{
public:
  incoming_message();
  ~incoming_message();
};

////
class outgoing_message
  : public rec_outgoing_data
{
public:
  outgoing_message();
  ~outgoing_message();
};
#endif

//transmission.cpp
# include "msg_transmission.h"
incoming_message::incoming_message()
{
  persistent::registerClass("rec_incoming_data",
     rec_incoming_data::rec_incoming_data_class);    // whala .. register the class
   // fill in holes - if any here.

}

incoming_message::~incoming_message()
{
 
}

outgoing_message::outgoing_message()
{
   // fill in holes - here.
}

outgoing_message::~outgoing_message()
{
    //
}


##############################################

1)   data_stream (not shown) is what it is from before..  No changes!!!   persisten.h & cpp are for the most part the same.  The prime difference is a pure virtual exeute was added to persistent.h

What I'm asking now is.  Can you fill in the holes (add in source code) for me.   More specifically:

A))))

The xmit member function in comm_rec that - according to our initial design -  will
1.  'store' the stream
2.   get the buffer and
3.   send data. (msocket->WriteBlock (buf, size));  // This might be best suited for the outgoing_message class in persistent.  Although I'd pefer not to clutter up peristent with this writeBlock mess.

B))))
The receive() member function in comm_rec - that according to our initial design - will
1.  convert 'mBuffer' into a data stream.
2.  Map stream object into the 'real' (rec_incoming_data) stuct.

The implementation of the execute member fucntion is still a mystery to me... so again just fill  in the holes (put in source code) for the incoming_message and outgoing_message class for me coupled with the execute function.

Admittidely, this approach is a lot cleaner.   This will definately put me back in business.

I wont be able to respond till  later this evening (3/4 ish ET) ... got to rush off on a trip here.

Thanks.

 
0
 

Author Comment

by:forums_mp
ID: 13950772
Alex, I suspect you're probably busy but with regard to my previous post, let me know.
Would like to potentially finish testing today if I could.
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 13957508
>>>> I suspect you're probably busy

Yes, I had a busy weekend organizing and feasting a big party. So, I had no time the last days and little time today cause I need to do my book-keepings.

I'll try to answer your previous post later this day.

Regards, Alex


0
 

Author Comment

by:forums_mp
ID: 13958820

I envisioned you're busy.  Mothers day and all :).   Appreaciate it.  We're close so I think once i nail this approach, all's well.
.
0
 
LVL 39

Assisted Solution

by:itsmeandnobodyelse
itsmeandnobodyelse earned 2000 total points
ID: 13961042
#ifndef DATA_STREAM_H
#define DATA_STREAM_H

#include <iostream>
#include <string>
#include <map>
using namespace std;

struct data_stream
{
  string classId;
  string buffer;
  int    curPos;

  data_stream(string clsId)
    : classId(clsId), curPos(0)
  {
  }
 // all the data_stream stuff
  bool isValid() { return curPos <= buffer.length(); }
};

template <class T>
data_stream& operator<<(data_stream& ds, const T& t)
{
    int siz = sizeof(T);
    string s(siz, ' ');
    memcpy(&s[0], &t, siz);
    ds.buffer += s;
    ds.curPos += siz;
    return ds;
}

template <class T>
data_stream& operator>>(data_stream& ds, T& t)
{
    int siz = sizeof(T);
    if (ds.curPos + siz <= ds.buffer.length())
        memcpy(&t, &ds.buffer[ds.curPos], siz);
    ds.curPos += siz;
    return ds;
}

#ifndef PERSISTENT_H
#define PERSISTENT_H

# include<map>
# include<string>
//# include "data_stream.h"  /// data_stream hasn't changed

class persistent;
typedef persistent* (*CreateInstanceFunc)();
struct data_stream;

class persistent
{
  // factory map is mapping class names to create functions
  static std::map<std::string, CreateInstanceFunc> factoryMap;
public:
  virtual std::string    getClassId() = 0;
  virtual bool           loadFromStream(data_stream& ds) = 0;
  virtual bool           storeToStream(data_stream& ds) = 0;
  virtual void           execute() = 0;
 
  static persistent* createInstance(const data_stream& ds);
   
  static bool registerClass(
        const std::string& className,
         CreateInstanceFunc createFunc
       );
   
  data_stream*  store();
};

#endif

// transmission.h

#ifndef MSG_TRANSMISSION_H
#define MSG_TRANSMISSION_H

# include<map>
# include<string>
//# include "persistent.h"
//# include "data_stream.h"

struct rec_data   //  message to comm_rec
   : public persistent
{
  int          i;
  double       d;
  std::string  s;
  std::string  s1;
  std::string  s2;
  bool         b;
  char         c;    

  rec_data()
    :  i(1), d(2.), s("abc"), s1("def")
       , s2("ghi"), b(false), c('Z')
  {
  }

  virtual ~rec_data() {}

  std::string getClassId() {
    return "rec_data";
  }
  bool loadFromStream(data_stream& ds) {
    ds >> i >> d >> s >> s1 >> s2 >> b >> c;
    return ds.isValid();
  }

  bool storeToStream(data_stream& ds) {
    ds << i << d << s << s1 << s2 << b << c;
    return ds.isValid();
  }

};

struct rec_incoming_data   // incoming message to comm_rec
   : public rec_data
{

  //static bool registered;
  static persistent* rec_incoming_data_class() { return new rec_incoming_data; }

  void execute()
  {
      // What happens here ?????        

      // You had a code snippet where you tried to make a
      // dynamic_cast on the persistent pointer you got from recv(...)
      // Instead you should call persistent::execute function that
      // would virtually call rec_incoming_data::execute()

      // Here you don't need a cast

  }

};

struct rec_outgoing_data   // outgoing_msg FROM comm_rec
  : public rec_data
{

  void execute()
  {
    // REALLY NOT NEEDED BUT
   // HAVE TO HAVE SINCE WE DERIVE OFF PERSISTENT

      // yes, that's true.
  }

};

/////////////////////////////////////////////////////////////////////////////////
// >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
// Actually, you don't need the following

// now we're ready
class incoming_message
  : public rec_incoming_data
{
public:
  incoming_message();
  ~incoming_message();
};

////
class outgoing_message
  : public rec_outgoing_data
{
public:
  outgoing_message();
  ~outgoing_message();
};
#endif

//transmission.cpp
// # include "msg_transmission.h"
incoming_message::incoming_message()
{
  persistent::registerClass("rec_data",
     rec_incoming_data::rec_incoming_data_class);    // whala .. register the class
   // fill in holes - if any here.

}

incoming_message::~incoming_message()
{
 
}

outgoing_message::outgoing_message()
{
   // fill in holes - here.
}

outgoing_message::~outgoing_message()
{
    //
}

#endif


Note:

1. Use a baseclass rec_data to not having to duplicate data members

2. The execute function is to use virtuality after creation of the persistent pointer via factory.
    In the execute function, 'this' pointer is of type rec_incoming_data. You don't need a cast.

3. The classId is an identification of the data *and not* of the create function. The data_stream
    buffer contains the ID and makes a lookup in the factory map whether there is a create function
    that applies to that ID. The ID was written by the sender class and must be "rec_data" in your
    case.

4. The derived classes incoming_message and outgoing_message were not needed. However, if
    you think you need them, you would have to provide the create function that was associated to
    "rec_data" when registering. It is always the most derived class which needs to register it's
    create function and associate it to the class id of the data baseclass.

5. The sender doesn't need to register the outgoing class. Only the receiving class must be registered.

6. Don't register in constructor of incoming class. You need registering prior to creating first object.
    Better use a global bool variable before main() to register all factory classes:

       // registering is prior to main
       bool g_rec_data_registered = persistent::registerClass
                                                  ("rec_data", rec_incoming_data::rec_incoming_data_class);
       int main()
       {
            ....
       }
         

Regards, Alex
0
 

Author Comment

by:forums_mp
ID: 13961412

Alex, thanks for the response.  One more thing on this execute:

since 'rec_data' is derived off 'persistent', 'rec_data' needs an 'execute' member function.   That said, I'm confused on why 'rec_incoming_data' needs an 'execute' function since 'rec_data' is the one derived off 'persistent'.    OR should  the execute member function be moved from persistent to 'rec_data', but that doesnt seem to make sense.

I need to pull out my C++ text here to refresh on inheritance but I guess rec_data and rec_incoming_data will both have execute member functions but the one in rec_data does nothing..  Seems odd
 
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 13961882
rec_incoming_data is derived from persistent as well!!

          persistent                        // baseclass helper
                |
          rec_data                          // data baseclass (gives the identifier)
                |
          rec_incoming_data           // most derived class (gives the create function)
                                                // here execute function has a 'this' of type rec_incoming_data
                                                // that exactly is the type the factory has created

>>>> but I guess rec_data and rec_incoming_data will both have execute member functions

No, rec_data has no execute as there are *never* rec_data instances neither at the sender's site nor at the receiver's site.

Regards, Alex
0
 

Author Comment

by:forums_mp
ID: 13964222

Alex,  hate to bother you with this but it's execute again.  You wrote:

  void execute()
  {
      // What happens here ?????        

      // You had a code snippet where you tried to make a
      // dynamic_cast on the persistent pointer you got from recv(...)
      // Instead you should call persistent::execute function that
      // would virtually call rec_incoming_data::execute()

      // Here you don't need a cast
  }


////////////////////////// (B)
Here's the code snippet you're referring to:

void CommRecorder::slotRead()
{
  int recv_msg_size = mSocket->readBlock(mBuffer, 10000);

  data_stream stream(mBuffer, recv_msg_size);

  if (stream)
  {
    my_data_class *ptr_msg =
        dynamic_cast<my_data_class*>(persistent::createInstance(stream));    // (99)
    if (ptr_msg)
    {
      if (ptr_msg->loadFromStream(stream))     /// (200000)
      {
      } else {

       }
    }
    else {
      //cout << " returning false " << endl;
     }
  }


///////////////////////////////////// (C)
Now based on your comment here's the revised slotRead as I understand it

void CommRecorder::slotRead()
{
  while (( rvcdMsgSize = socket->receive(echo_buff, RCVBUFSIZE)) > 0)  
  {
    cout << rvcdMsgSize << endl;                          
    data_stream stream(echo_buff, rvcdMsgSize);    
    if (stream)
    {
      persistent* p = createInstance(stream);            /// (1)
      if (p != NULL)                                                  /// (2)
      {                
        p->execute();   // that function does the job     /// (3)
        delete p;                                                       /// (4)
      }

   }
}

The lines marked (1) to (4) is borrowed from a previous post of yours.   For starters should item 1 be my

With item 1.  I'm no longer doing the dynamic cast hence I still don't undertand what code should go inside the execute function ( i understand the virtual call and all but .. what shoul execute do)?  

Also , what happens to the line marked "(200000)".  After the call to execute in slotRead, How do I get visibility into the data within slotRead so I can do something akin to what I was previously doing.  i.e

        if (ptr_msg->loadFromStream(stream))          // (200000)
        {
          cout << " i " << ptr_msg->i << endl;   // print the data
          cout << " d " << ptr_msg->d << endl; // print the data
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 13965647
(1) zo (4) is perfect.

>>>> hence I still don't undertand what code should go inside the execute function

You should do that what you had done if you would have got the pointer by a dynamic_cast:

 void CommRecorder::slotRead()
{
  while (( rvcdMsgSize = socket->receive(echo_buff, RCVBUFSIZE)) > 0)  
  {
    cout << rvcdMsgSize << endl;                          
    data_stream stream(echo_buff, rvcdMsgSize);    
    if (stream)
    {
      rec_incoming_data* p = (rec_incoming_data*)createInstance(stream);            /// (1)
      if (p != NULL)                                                  /// (2)
      {                
        //  What had you done here ????                     /// (3)
        delete p;                                                       /// (4)
      }
   }
}

You could enhance the execute function by an argument - say a pointer to CommRecorder. That would allow you to go back to CommRecorder in execute.

  void rec_incoming_data::execute(CommRecorder* pRecorder)
  {
       pRecorder->doWhatHasToBeDone(this);
  }

However, OO design should/could allow the objects to act themselves rather than be driven by a controller.

  void rec_incoming_data::execute(CommRecorder* pRecorder)
  {
      storeToDatabase();
      showData();
      pRecorder->updateAllViews(this);
  }

Regards, Alex



   
0
 

Author Comment

by:forums_mp
ID: 13966882


With regard to your question:
>>>>>>       //  What had you done here ????                     /// (3)

What I did was call 'loadFromStream'.

        if (p->loadFromStream(stream))          // (3)
        {
          cout << " i " << ptr_msg->i << endl;
          cout << " d " << ptr_msg->d << endl;
          cout << " s " << ptr_msg->s << endl;
          cout << " b " << ptr_msg->b << endl;
          cout << " c " << ptr_msg->c << endl;
        } else { cout << " loadFromStream returned false " << endl; }

The call to loadFromStream is necessary right so ...
The ONLY thing left at that point is 'replacing' the 'couts' with the ACTUAL code to update the 'widgets' on the dialog.
With the execute approach, the 'cout's (ultimately I'll replace couts with ACTUAL code to update widgets) would be moved to execute. Ideally - dont you think it's simpler to just keep all the 'updating' in CommRecorder (- just like above in the slotRead member function?

I think we've solidified the design :).  One other thing ..   I suspect I could change this
 rec_incoming_data* p = (rec_incoming_data*)createInstance(stream);            /// (1)
to use a reference instead?

Trying to alleviate the need to  do item 4 below on a continuum.

    if (p != NULL)                                                  /// (2)
      {                
        //  What had you done here ????                     /// (3)
        delete p;                                                       /// (4)
      }
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 13968553
>>>> dont you think it's simpler to just keep all the 'updating' in CommRecorder

It might be simpler, but from a designer's view it is a big offense if a class of the database or communication layer (here CommRecorder) cares about outputting data in a view what is on a layer that should be at least two layers below:

     communication layer
     database layer
     application layer
     visual layer

Note, any of these layers should have an interface to the neighbor layers. An interface means that there is data that goes from one layer to the next, but no class on any layer is allowed to drive classes of another layer.

By calling execute() you either could visualize by using cout calls or by using a GUI. The loadFromStream is a data interface only. Also the factory is anonymous - i. e. the communication layer doesn't know what kind of classes (on what layer) were created. It simply calls execute and the visualization is independent. So, if you later need to visualize the data at a palm or smart phone, derive a new class, override the execute function and change the registerClass call, and all is done.

>>>>  I suspect I could change this
>>>>  rec_incoming_data* p = (rec_incoming_data*)createInstance(stream);            /// (1)
>>>> to use a reference instead?

Actually, createInstance needs to return NULL if the classId wasn't registered. That isn't possible when returning a reference. I think delete p isn't that bad. Or you could store all new object pointers to a container class (in execute) that would be responsible to delete all at program end (or at request).

Regards, Alex






 
0
 

Author Comment

by:forums_mp
ID: 13970218

[QUOTE]
Note, any of these layers should have an interface to the neighbor layers. An interface means that there is data that goes from one layer to the next, but no class on any layer is allowed to drive classes of another layer.
[/QUOTE]

I see!!.  I've got a lot to learn when designing classes.  Can't you tell?  :)

[quote]
The loadFromStream is a data interface only.
[/quote]

So are you saying that  (3) below is not necessary on the receive side of things?

      rec_incoming_data* p = (rec_incoming_data*)createInstance(stream);       // (1)
      if (p != NULL)      
      {
        if (p->loadFromStream(stream))          // (3)
        {
          cout << " i " << ptr_msg->i << endl;
          cout << " d " << ptr_msg->d << endl;
          cout << " s " << ptr_msg->s << endl;
          cout << " b " << ptr_msg->b << endl;
          cout << " c " << ptr_msg->c << endl;
        }
      }

Makes sense since theoretically I could just work with the data from (1).

//////////////  Side item

 vector<int*> p_vec;
 for (int idx(0); idx < 5; ++idx)
 {
   int *p = new int[15];    // (99)
   p_vec.push_back(p);
 }

// some destructor...
 for (size_t idx(0); idx < p_vec.size(); ++idx)
 {
    delete p_vec[idx];
 }

The line marked 99 dynamically allocates memory on 5 times, so in essence p points to a new location with each occurance.  So far so good?  
In order fo rme to delete p I push_back p within a vector (i'm starting to like this container:)).  The question .. that's the 'ideal' approach typically?
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 13970894
>>>> So are you saying that  (3) below is not necessary on the receive side of things?

Yes, but you need to move the loadFromStream call to persistent::createInstance. Then, the object returned by pointer is already filled with data.


>>>> vector<int*> p_vec;

Generally a vector of pointers isn't a good idea as the destructor of std::vector wouldn't free the memory allocated by new. If you really need integer arrays with exactly 15 elements, you should make a struct

  struct IntArr15
  {
        int arr[15];
        int& operator[] (int idx) { if (idx < 0 || idx >= 15) return arr[0]; return arr[idx]; }
  };

or a class

   class IntArr15 : public std::vector<int>
   {
   public:
          IntArr15() : vector<int>(15) {}  
   };

or even

   typedef int IntArr15[15];

All of these would properly manage it's storage when creating, when copying and when destructing.

>>>>    delete p_vec[idx];

You would need

    delete [] p_vec[idx];  


>>>> In order for me to delete p I push_back p within a vector

No, std::vector makes a copy of a pointer that was inserted by push_back but it never deletes pointers. You would need to make an own pointer class to achieve that.

class IntArr15Pointer
{
       int* pt;
public:
       IntArr15Pointer() : pt(new int[15]) { }
       ~IntArr15Pointer() {  delete [] pt; }

};

But you better would take any of the other approaches.

Regards, Alex
0
 

Author Comment

by:forums_mp
ID: 13973997
Alex, I'm going to shift gears for a second here.  With respect to the use of vectors, how would you modify code below to use a vector instead of 'record_array'.


struct RECORD {
  int byte_count;
  int destination_address;
  int source_address;
  int prt_next_record;
};

# define REC_ADDRESS 0xe000000

void test(unsigned int *dest_addr,
             unsigned int *source_addr,
           unsigned int block_size)
{
  RECORD *record_array;
  record_array = (RECORD*)REC_ADDRESS;

  while (block_size  < 0xa00000 + 1)
  {
    record_array[0].byte_count            = (unsigned int) block_size;
    record_array[0].destination_address  
       = (unsigned int) (dest_addr +  block_size );
    record_array[0].source_address  
       = (unsigned int) (source_addr +  block_size );
    record_array[0].prt_next_record      = (unsigned int)&record_array[1];
            
    record_array[1].byte_count            = (unsigned int) block_size;
    record_array[1].destination_address  
        = (unsigned int) (dest_addr +  (2 * block_size) );
    record_array[1].source_address  
        = (unsigned int) (source_addr + (2 * block_size) );
    record_array[1].prt_next_record      = (unsigned int)&record_array[2];

    record_array[2].byte_count            = (unsigned int) block_size;
    record_array[2].destination_address  
        = (unsigned int) (dest_addr +  (3 * block_size) );
    record_array[2].source_address  
        = (unsigned int) (source_addr +(3 * block_size) );
    record_array[2].prt_next_record      = (unsigned int)NULL;

    if (block_size < 0x80000)
      block_size = 0x80000;
    else
      block_size += 0x80000;
  }
}
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 13975563
You are trying to do Assembler using C++ ??????

Actually. I couldn't imagine any purpose where the code above would make sense.

>>>> # define REC_ADDRESS 0xe000000

Where did you get that address? Why not simply allocate local or global memory? Or shared memory if you need to share with other processes?

>>>> unsigned int *dest_addr
>>>> unsigned int *source_addr

These arguments are defined as unsigned int pointers. The arithmetic expression

     dest_addr + blocksize

is equivalent to

    ((unsigned int)dest_addr) + blocksize*sizeof(unsigned int)

That means you are adding blocksize * pointer size to the initial address. Did you know that?

Why are you casting pointers to non-pointers and vice versa? There isn't any need for that. If you want to compute pointers - for whatever reason - use pointers and do not mix it up with integer arithmetics.

>>>> while (block_size  < 0xa00000 + 1)

If I counted correctly the loop will run 20 times (if the initial blocksize is less equal to 0x80000). If so, why not simply using a for loop running from 0 to 20? Why passing an argument blocksize that actually *has to be* 0x80000?

>>>> record_array[0], record_array[1], record_array[2]

The loop doesn't make sense. You overwrite the same values of the first three record_array elements with every run of the loop. Finally you have

      record_array[0].byte_count            =  0xa00000;  // (unsigned int) block_size;
      ...

and all previous assignments have been made for nothing.

Regards, Alex






0
 

Author Comment

by:forums_mp
ID: 13982150

Alex, you picked this code apart.  The code was taken from an example application provided by a vendor.   I'm glad I'm  not the only one perusing the code going huh? :)

[quote]
>>The loop doesn't make sense. You overwrite the same values of the first three record_array elements with every run of the loop. Finally you have

record_array[0].byte_count            =  0xa00000;  // (unsigned int) block_size;

[/quote]

I agree with you.  The only thing that changes is block size.    here's the core of the initial source that compiles.  Just add 'RECORD' above + the body of the while loop here.



# define REC_ADDRESS       0xe000000
# define SOURCE_ADDRESS 0xDF000000
# define DEST_ADDRESS     0xe000000

void test()
{

  RECORD *record_array;
  unsigned int   block_size, data_size;
  unsigned int *dest_addr, *source_addr;

  block_size = 0x1000;
  data_size  = 4 * block_size;
  source_addr = (unsigned int *)SOURCE_ADDRESS;
  dest_addr    =  (unsigned int *)DEST_ADDRESS;
  record_array  = (RECORD *)REC_ADDRESS;

  while ( block_size < (0xa0000 + 1))
  {
    // stuff from above


    memset(source_addr, 0x55, data_size);
    // perform the dma
    // dma_write( (unsigned int) source_addr, (unsigned int)dest_addr, block_size, record_array[0]);
    if (block_size < 0x80000)
      block_size = 0x80000;
    else
      block_size += 0x80000;

    data_size = 4 * block_size;

  }
}

So what i need to do is rewrite the source... Get user to pass in source, destination address AND block size.

Check to see if block size is greater than 16 MiB.  I can only transfer/pass in  16 MiB blocks to dma_write.  'dma_write' along with 'cacheMalloc' & 'cacheFree) is a vendor 'thing' and commented out;

So now:

  void test ( size_t size, size_t *dest_addr, size_t *source_addr)
  {
    size_t     tx_bytes(0);
    RECORD *rec_chain;

    int num_records(0);
    const int MAX_TRANSFER_COUNT(16 * 1024 * 1024);

    if (size > MAX_TRANSFER_COUNT )
      num_records = (size / MAX_TRANSFER_COUNT ) + 1;

    else
      num_records = 1;
 
   //rec_chain = (DMA_RECORD*)cacheMalloc(
   //                            sizeof (DMA_RECORD) *
   //                            num_records
   //                          );

  bool building_chain(true);

  while (building_chain)
  {
    if (size > MAX_TRANSFER_COUNT)  {
      tx_bytes = MAX_TRANSFER_COUNT;
      size  -= MAX_TRANSFER_COUNT;
    }
    else {
      tx_bytes = size;
      building_chain  = false;
    }
 
    for (int idx(0), jdx(1); idx < num_records; ++ jdx, ++idx)
    {
      rec_chain[idx].ByteCnt    = tx_bytes;
      rec_chain[idx].DestAdd    = (size_t)dest_addr   + (jdx * tx_bytes);
      rec_chain[idx].SrcAdd     = (size_t)source_addr + (jdx * tx_bytes);

      if ( idx == ( num_records - 1) )
      {
        rec_chain[idx].NextRecPtr = 0;
      }
      else {
        rec_chain[idx].NextRecPtr  = (size_t)&rec_chain[jdx];

      }
      ++jdx;
    }
  ///
    // dma_write( (unsigned int) source_addr, (unsigned int)dest_addr, tx_bytes, record_array[0]);
  }
  // cacheFree (rec_chain);
}

How does that look?  Re-write where necessay .. thanks!!

0
 

Author Comment

by:forums_mp
ID: 13983217

Alex, I just realized I goofed on the above.   In a chain mode DMA operation, all I need to do is setup the records based on the number of bytes (size).  ie.

    if (size > MAX_TRANSFER_COUNT )
      num_records = (size / MAX_TRANSFER_COUNT ) + 1;

    else
      num_records = 1;

So if num_records = 2.
   then I end up with 2 record arrays.

   record_array[0].byte_count            = (unsigned int) block_size;
    record_array[0].destination_address  
      = (unsigned int) (dest_addr +  block_size );
    record_array[0].source_address  
      = (unsigned int) (source_addr +  block_size );
    record_array[0].prt_next_record      = (unsigned int)&record_array[1];
         
    record_array[1].byte_count            = (unsigned int) block_size;
    record_array[1].destination_address  
       = (unsigned int) (dest_addr +  (2 * block_size) );
    record_array[1].source_address  
       = (unsigned int) (source_addr + (2 * block_size) );
    record_array[1].prt_next_record      = 0;

All that while stuff was there just to illustrate back to back transfers.
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 13984714
Still have doubts that you should need a record_array instead of a single record. If I remind correctly from your previous thread only single records were passed to the interface.

Also, by adding block_size to a pointer you are actually adding (block_size * pointer size) to the address. I hardly can believe that this is intended.

>>>> All that while stuff was there just to illustrate back to back transfers.

??? Looks more like anyone tried to remain indispensable ;-)

Regards, Alex



0
 

Author Comment

by:forums_mp
ID: 14006350
Alex, two questions.
Related - indirectly to a block size response you made in an earlier post.

Consider:

   const int buffer_size(15 * 1024 * 1024);

   int *send_buffer = new int[buffer_size];   // allocate a 15 MiB buffer  (1)
   // later.  Fill the buffer
   for (int idx(0); idx < buffer_size; ++idx)
      send_buffer[idx] = idx;

  // perform Some DMA operation on the send_buffer
  // dma_op->perform_dma( send_buffer, destination, buffer_size );

Where send_buffer is the source, destiation is the destination address and buffer_size is just that.

The receiver receives the data and alls well.  Now when I examine the data on the disks.  Here's what it looks like.  

#########################################################
Block Number: 0x00000000, Block size: 1024 B
00000000: 00000000 00000001 00000002 00000003 00000004 00000005 00000006 00000007
00000008: 00000008 00000009 0000000a 0000000b 0000000c 0000000d 0000000e 0000000f
:
:  // and so on up to
000000f0: 000000f0 000000f1 000000f2 000000f3 000000f4 000000f5 000000f6 000000f7
000000f8: 000000f8 000000f9 000000fa 000000fb 000000fc 000000fd 000000fe 000000ff

#########################################################
Similarily @ block number 0x00003bff, I examine the data and ..

Block Number: 0x00003bff, Block size: 1024 B
00000000: 003bff00 003bff01 003bff02 003bff03 003bff04 003bff05 003bff06 003bff07
00000008: 003bff08 003bff09 003bff0a 003bff0b 003bff0c 003bff0d 003bff0e 003bff0f
:
:  // and so on up to
000000f0: 003bfff0 003bfff1 003bfff2 003bfff3 003bfff4 003bfff5 003bfff6 003bfff7
000000f8: 003bfff8 003bfff9 003bfffa 003bfffb 003bfffc 003bfffd 003bfffe 003bffff

////////////////////////////////////////////////////////////////////////////////////////
Now whats interesting is.  Block Number (3bFF + 1) * Block Size (1024) = 15 MiB.   Trouble is the last value I would expect to see is 15728639 (buffer_size - 1) NOT (0x003bffff) 3932159 decimal.

I'm off by a factor of 4 (15 MiB / 3932159 + 1  =  4) and I'm not sure what I'm missing.

// so in summary
int *send_buffer = new int [15 MiB];
  for (int idx(0); idx < 15 MiB; ++idx)   // fill the buffer with a counting pattern
   send_buffer[idx] = idx;        

Now send_buffer = 60 MiB - if sizeof int = 4 (which it is).  On a send operation, I send:  send_buffer + buffer_size (15 MiB).
Now upon examining the send_buffer on the receive side , I expect the last value to reflect (15 MiB - 1 - based on the for loop) but it doesn't.  

Instead 15 MiB/4 (003bffff)  - 1  is the last value.   Come to think of it.  I think where I'm mixing things up is this.  The length and the allocation size of the buffer are obviously 2 independent items.  The 'buffer_size' is not 15 MiB.   The actual size is 15 MiB * 4 (sizeof int).  = 60 MiB.  When pass in 15 MiB for 'a size'.   I'm getting 1/4 of the 15 MiB.  THis seems like a complete waste.  IOW.  I REALLY allocate 60 MiB based on a 15 MiB length.   Why not just allocate
 int *p = new int [0x3c0000];
 
Now sizeof(int) * 0x3c0000 = 15 MiB.    // buffer_size here = 0x3c0000
Transmit p via dma like so:
  // dma_op->perform_dma( p, destination, sizeof(p));

Note: parameter three is the sizeof the entire buffer. Except there's one problem.  sizeof (p) is the size of a pointer so what would be better then
  // dma_op->perform_dma( p, destination, buffer_size * sizeof (int));

Makes sense and preferred?


//////////////////////////// 2
If one were to use a 'integer' value as opposed to a string for the registerClass.  What impact would that have?  A friend of mine likes this idea but would like to  keep his existing ids (which are integers).

 Persistent::registerClass("outgoing_msg", outgoing_msg::createMsg);

//////////////////////////// 3.
How would you implement the checksum (simple addition of all the data) parameter of the 'header' recall header is:
struct header
{
  unsigned short size;
  unsigned short id;
  unsigned short count;
  unsigned short chksum;
};
0
 

Author Comment

by:forums_mp
ID: 14006526

Alex, one other thing:

With respect to your comment:

 "Also, by adding block_size to a pointer you are actually adding (block_size * pointer size) to the address. I hardly can believe that this is intended."

So If you're trying to put together three records based on the record struct.

// allocate memory for the three records...
// later
    record    [0 ].source_address   = (int)source_address_passed_in  + blocksize;
    record    [1 ].source_address   = (int)source_address_passed_in  + blocksize * 2
    record    [2 ].source_address   = (int)source_address_passed_in  + blocksize * 3

All they're doing in the code is adding in an offset.

Based on your comment, how would you approach that differently?  I'd like to improve my approach and not effictively mirror theirs.
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 14006880
>>>>    int *send_buffer = new int[buffer_size];   // allocate a 15 MiB buffer  (1)

You allocated a 60 MB buffer cause you requested 15 MB integers each sized 4 bytes. That's all correct and I couldn't see any waste. Look at this:

struct c100
{
    char c(100);
};

 c100* p = new c100[1000];

Of course allocation size is 100 000 bytes and not 1000!!

So you always need to consider the individual size of the types you are requesting with new. Note, that is where 'new' differs from 'malloc'. With malloc you are requesting bytes always (but you need a cast then).    

>>>> record [0 ].source_address   = (int)source_address_passed_in  + blocksize;

By casting source_address_passed to int the following plus operation is turned to an integer addition. Without casting it was a pointer addition that would add in portions of the type the pointer was pointing to, here an int. Though the rules are clear, it is not only you that has difficulties to not mixing up things. Therefore I hold up my recommendation to avoid that. You shouldn't pass the variable as a int pointer if you need to handle it as an integer address. If you got a pointer by a third-party interface, write a wrapper that turns pointers to int - once - and don't care about weird interfaces. Check, if you could avoid address arithmetics by simply requesting only one record at each call (not an array).

Regards, Alex

0
 

Author Comment

by:forums_mp
ID: 14007464
Alex, I'm trying to finalize things here so I could move beyond 'our' persistent class :)  On 05/10/2005 10:56AM PDT you offered up the response based on my question

>>>> So are you saying that  (3) below is not necessary on the receive side of things?

Yes, but you need to move the loadFromStream call to persistent::createInstance. Then, the object returned by pointer is already filled with data.
-------------------------------------------------------------------------------------------------------------------------------------
I'm having some difficulty understanding that ONLY because I'd like the fact that persistent is generic at present.  'That sad, I could use persistent with ALL different data types.  If I add a loadFromStream function within persistent createInstance then I loose the generic 'ness' of persistent.  

Having said that.   Here's where we are as I understand things:

# include <iostream>
# include <vector>
# include "data_stream.h"
# include "persistent.h"
using namespace std;

struct ic_data   // incoming message to comm_rec
   : public persistent
{
  int          i;
  double       d;
  std::string  s;
  std::string  s1;
  std::string  s2;
  bool         b;
  char         c;    

  ic_data()
    :  i(1), d(2.), s("abc"), s1("def")
    , s2("ghi"), b(false), c('Z') {}

  virtual ~ic_data() = 0 {}

  std::string getClassId() {
    return "incoming";
  }
  bool loadFromStream(data_stream& ds) {
    ds >> i >> d >> s >> s1 >> s2 >> b >> c;
    return ds.isValid();
  }

  bool storeToStream(data_stream& ds) {
    ds << i << d << s << s1 << s2 << b << c;
    return ds.isValid();
  }
};


struct incoming_data  
   : public ic_data
{
  static bool registered;
  static persistent* create_msg() { return new incoming_data; }
  void execute()  // will need to pass in the this pointer of object
  {
   // Might end up calling a showData here ..
   // Will look more into this
    //pRecorder->showData(this);
  }
};



struct og_data   // outgoing message to comm_rec
   : public persistent
{
  int          i;
  double       d;
  std::string  s;
  bool         b;
  char         c;    

  og_data()
    :  i(0), d(5.), s("hijk"), b(false), c('A')  {}

  virtual ~og_data() = 0 {}

  std::string getClassId() {
    return "og_data";
  }
  bool loadFromStream(data_stream& ds) {
    ds >> i >> d >> s >> b >> c;
    return ds.isValid();
  }

  bool storeToStream(data_stream& ds) {
    ds << i << d << s << b << c;
    return ds.isValid();
  }
};

struct outgoing_data  
   : public og_data
{
  void execute()
  {
    // REALLY NOT NEEDED BUT
    // HAVE TO HAVE SINCE WE DERIVE OFF PERSISTENT
    // yes, that's true.
  }
};


bool incoming_data::registered  =
  persistent::registerClass( " incoming_data ",  
                                 incoming_data::create_msg);   //(1)
int main()
{

}

void send()  // sending now looks like
{
  outgoing_data od;

  od.i = 99;
  od.d = 9;
  od.s = "testing";
  od.b = true;
  od.c = 'A';

  data_stream* pds = od.store();
  char *buf(0);
  int  size(0);

  if ((size = pds->getBuffer(buf, 0)) > 0)
  {
    // send buffer
    //mSocket->writeBlock(buf, size);
  }

}

void receive() // receiving now looks like
{
  char echo_buff[4096];
  int rvcdMsgSize(0);

  //while (( rvcdMsgSize = socket->receive(echo_buff, RCVBUFSIZE)) > 0)  
  //{
    data_stream stream(echo_buff, rvcdMsgSize);    
    if (stream)
    {
      incoming_data* p = (incoming_data*)persistent::createInstance(stream);  
      if (p != NULL)                                      
                  {                
        //p->execute();   // that function does the job     /// (3)
        delete p;                                                  
      }
   }
  //}
}


////////////////  
With regard to the send and receive functions.  Not much has changed.  Some comments:

The commented out showData function resident within the execute member function of the incoming_data 'struct' is the approach I'll use to update the 'widgets' of the dialog.    Admittiedley it appears odd to me that I need to call an execute member function (item 3) above when I could just operate on the data right there  with the 'p' pointer.  I've come to like the approach though after reading your explanation some posts ago.

My concern now though is:
1.  I'm not sure what you mean by adding LoadFromStream to createInstance.  Tweak createInstance (from any of our persistent classes) so I see what you mean but adding loadfromStream to createInstance.

2.  og_data (og - outgoing) and ic_data (ic - incoming) class both have loadFromStream storeToStream, yet - in theory - correct me if I'm wrong,  ic_data cares ONLY about loadFromStream.  Likewise, og_data ONLY cares about 'storeToStream'.

3.  Should I move loadFromStream and storeToStream to the derived struct of ic_data  and og_data.   ie. should I move Load and Store to incoming_data and outgoing_data.   I think the answer is NO but just checking.

4.  With respect to the incoming_data and outgoing_data structs.  Would you agree that's all the member functions I need?  If yes, then the struct incoming_data (it only has an execute member function which I dont need but have to have)seems useless.  Agreed?

5.  To prevent the user from creating instances of og_data and ic_data.  I made the destructors pure virtual.  
  virtual ~og_data() = 0 {}
  virtual ~ic_data() = 0 {}
Correct approach?

Feel free to tweak code as you see fit.  This makes it easier and cuts down on my potentially silly questions.  Thanks

Ultimately (next round - new post perhaps) I'll get you to help me on one additional streamline of the source.  I think it'd be prudent to not have to offer up two execute member functions when only one struct cares abotu such function.  I believe we could tweak code to achieve that.   For now though I want to get as close to 'finishing' this current version.  Then again, you're pretty quick so .. I'll defer to you if you see a immediate solution with regard to a single execute function.


0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 14014906
>>>> If I add a loadFromStream function within persistent createInstance then I loose the generic 'ness' of persistent.

No, you would create persistent pointers by factory and call loadFromStream virtually.

>>>> std::string getClassId() {  return "incoming"; }

No, no, no. The classID of sending *must* be identical to that of receiving, so a name like 'incoming' is definitively wrong. The purpose of classID is to identify the data and is independent of any direction.

>>>> static bool registered;

That is dangerous as the factory map is a static class member as well. Unfortunately there is no way to  determine the order of creation of static members if they arfe defined in  different cpp files. I would recommend not to rely on static data members but have a static class member function (of persistent or some other manager class) called 'registerAllPersistentClasses' that registers all incoming classes. You would explicitly call that function in main() or any other initializing function that runs before any incoming messages could be received.

To (1)
As loadFromStream is a virtul function and the datastream object was passed to createInstance as an argument, it could/should called there. i. e. createInstance would return a incoming_data object already filled with data from datastream. You wouldn't need the cast if calling execute after getting the persistent pointer of createInstance.

To (2)
Indeed, outgoing_data only needs 'storeToStream' and incoming_data needs 'loadFromStream'. That would be different - and the names of the classes should be different then too - if the data would go to both directions, e. g. client_data and server_data.

To (3)
No, 'loadFromStream' and 'storeToStream' are data functions only and must not be overidden as long as there is only one derived class at each side.

To (4)
If you don't want to use execute you simply could remove it from persistent. However, you would loose the generic'ness of persistent if the only class you need to receive is 'incoming_data'. The exectute function is the means to have more than one object to transfer and generically automating the distribution. Also, if you don't want the execute function on both sides, simply make them not pure virtual. However, that might give bad errors at the receiving side if you once forget to provide an execute function.

To (5)
Though it may work, you should make private constructors rather than private destructors. A private constructor would allow the factory to create instances by (static) member function but noone else.

Regards, Alex
 





0
 

Author Comment

by:forums_mp
ID: 14015137

Granted theres room for improvements, things work (:)) and I've got to thank you again.  I'd like to truncate this post and continue the improvments in a 'part 2' thread.  I suspect I better work that with community services.

In a client server relationship.   On the GUI side (client) we have struct names akin to:
 
   struct gui_to_track {};     // outgoing
   struct track_to_gui {};     // incoming

On the server side (the track).  We have
    struct gui_to_track {};    // incoming
   struct track_to_gui {};     // outgoing

Now with regard to your comment item (1) above.  Within the 'receive' member function on either client or server I can't see any way around the 'casts' below:

server_side:
void server_side::receive()
{
  // later
  gui_to_track* p = (gui_to_track*)persistent::createInstance(stream);     // cast to the appropriate type
  p->execute();      
}

client_side - the GUI  in this case:
void client_side::receive()
{
  // later
  track_to_gui * p = (track_to_gui *)persistent::createInstance(stream);   // cast to the appropriate type
  p->execute();    
}

Indeed execute does not need the cast and this mirrors your proposed approach in your 05/09/2005 10:28PM PDT post.   Perhaps, I'm not understandign what you mean when you say 'dont need the cast'.   Could you illustrate with source code on this.

items (2 & 3):
So lets assume we have stucts client_data and server_data.  Here again on the CLIENT side, he has:

  struct client_data {};  // this is data sent to the server.
  struct server_data {}; // this is data received from the server

On the SERVER_SIDE

  struct client_data {};  // this is data received from the client.
  struct server_data {}; // this is data sent to the CLIENT

The struct client_data on the CLIENT side _ONLY_ needs storeToStream, but when putting it all together one must provide both load and store.  So now:

   struct client_data  : public persistent {
     
    bool loadFromStream(data_stream& ds) {   // really don't need here but MUST provide becasue it's pure virtual
       // stuff
       return ds.isValid();
     }
    bool storeToStream(data_stream& ds) {   // need this for transmitting data to the SERVER
       // stuff
       return ds.isValid();
     }

   }

Similarily struct server_data on the CLIENT side must provide a load and store.    Now for the SERVER side, things are reversed.  The struct client_data NEEDS loadFromStream and server_data struct needs storeToStream.

Having said that I'm not sure what you mean when you say
|  and must not be overidden

They're pure virutals hence how could I not override them?

Item (4).
| Also, if you don't want the execute function on both sides, simply make them not pure virtual

Perhaps, I'm misunderstanding pure virtual.  If you provide a pure virutal in a base class.  Derived classes MUST provide implmenentations - or at least that's what I thought.  

Now with deference to our structs server_data and client_data.  If I provide a pure virtual execute in persistent.  I must provide implementations in server_data and client_data.

So now - assume client side:
    struct client_data : public persistent  { // ougoing data to server
        void execute() {}  // DONT NEED it but must provide an implementation since it's pure virtual.
    }
    struct server_data : public persistent {  // incoming data from server
        void execute() {}  // here I could use execute      
   };

Server side is similar, except things again are reversed.  IOW:  The execute is need in the client_data struct and not needed in the server_data struct.  Making execute pure virtual results in providing an implementation in the derived classes.  Does it not?


/////////////////////
 I'd really like to put an end to this registered (static bool registered) thing once and for all.  Its a headache (been bitten a few times) :) and I'm not sure I understand this new solution you proposed:

| I would recommend not to rely on static data members but have a static class member function (of persistent or some
| other manager class) called 'registerAllPersistentClasses' that registers all incoming classes.

I like this idea of a static member function within persistent.  Just not sure how to approach it.


0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 14016520
>>>> ... on either client or server I can't see any way around the 'casts'

server_side:
void server_side::receive()
{
  // later
  persistent* p = persistent::createInstance(stream);     // cast to the appropriate type
  p->execute();  // In p->execute() 'this' is of correct type    
}

client_side - the GUI  in this case:
void client_side::receive()
{
  // later
 persistenti * p = persistent::createInstance(stream);   // cast to the appropriate type
  p->execute();      // In p->execute() 'this' is of correct type
}

The combination of C++ polymorphism and C function pointers gives a generic factory.

>>>> Making execute pure virtual results in providing an implementation in the derived classes.  Does it not?

Making execute virtual but not pure virtual would allow to have

  class persistent
  {
      ...
      void execute() {  error("Baseclass function never should be called"); }  
  };

Then, derived class may override execute if they needed.

>>>> So lets assume we have stucts client_data and server_data.  

If data can go to both directions, you need store and load at both sides, easiest to have same baseclass implementation for both function. Pure virtual functions must be overloaded but you must not have *pure* virtual (see sample before).

>>>> I like this idea of a static member function within persistent.  Just not sure how to approach it.

I would prefer to have the functionality on a 'manager' class - where you have a singleton that can be accessed evrywhere in your prog - rather than put the functionaölity to baseclass persistent.

//commmgr.h

class communication_manager
{
public:
     void registerAllPersistent();
};

// commmgr.cpp

void communication_manager::registerAllPersistent()
{
      incoming_data::registerClass("rec_data", incoming_data::create);
      // more ?
}

extern communication_manager theCommMgr;  // the one and only instance of communication_manager

//main.cpp


...

// instantiate the singleton in one cpp file only

communication_manager theCommMgr;  // the one and only instance of communication_manager

int main()
{
     theCommMgr.registerAllPersistent();
     ...
}


If you make sure that communication_manager is a singleton, you don't need a static function but a normal member would do it.

Putting the same functionality to a static function of persistent would work, but would be an offense to the principle of secretness that doesn't allow baseclasses to 'know' about derived classes.

Regards, Alex

0
 

Author Comment

by:forums_mp
ID: 14020046

| void client_side::receive()
| {
|   // later
|   persistenti * p = persistent::createInstance(stream);   // cast to the appropriate type
|   p->execute();      // In p->execute() 'this' is of correct type
| }


I see now.  I think what wasn't sinking in on my end is this.   Consider our earlier case:

struct rec_data  
  : public persistent           // incoming message to comm_rec
{
 int i;
 
};

void comm_rec::get_data()
 {
    // later
     data_stream stream(echo_buff, rvcdMsgSize);    

     rec_data *ptr_msg = (rec_data*)persistent::createInstance(stream);    // (11)
     if (ptr_msg)
      {
        if (ptr_msg->loadFromStream(stream))        
        {
          ////////////////////////////////////////
          //// NOW use i.
          int ll = ptr_msg->i;                                                                       // (12)
        }
      }
  }

Now to use the object belonging to rec_data (12), we'd cast as show in (11).

-----------------------------------------------------------------------------------------------------------------------------------------------------
With this 'new approach'.   We added a new struct.  

struct rec_incoming_data   // incoming message to comm_rec
   : public rec_data
{

  //static bool registered;
  static persistent* create_msg() { return new rec_incoming_data; }

  void execute()
  {
      // What happens here ?????        

      // You had a code snippet where you tried to make a
      // dynamic_cast on the persistent pointer you got from recv(...)
      // Instead you should call persistent::execute function that
      // would virtually call rec_incoming_data::execute()

      // Here you don't need a cast
  }
};

Except I think execute needs to look more like:

 void execute(comm_rec * ptr_comm_rec)
  {
      // What happens here ?????        

      // You had a code snippet where you tried to make a
      // dynamic_cast on the persistent pointer you got from recv(...)
      // Instead you should call persistent::execute function that
      // would virtually call rec_incoming_data::execute()

       ptr_comm_rec->updateAllViews(this);         /// (13)
  }

// Now get_data looks like

void comm_rec::get_data()
 {
    // later
     data_stream stream(echo_buff, rvcdMsgSize);    

     persistent* ptr_msg = persistent::createInstance(stream);    // (11)
     if (ptr_msg)
      {
         persistent->execute();
       }      
  }

// only now I need an 'updateAllViews' member function in comm_rec that'll allow me to use line (12) earlier.
void comm_rec::updateAllViews ( rec_incoming_data *rec )
 {
   ////////////////////////////////////////
   //// NOW use i.
   int ll = rec->i;                                                                       // (12b)
 }
 
Now comes my confusion (haven't had a chance to try this):

The call to execute from comm_rec's member function get_data needs to reflect
              persistent->execute(this);    Where 'this' is the address of the comm_rec object.   Correct?

The line marked 13 above is the 'this' of  rec_incoming_data, hence the updateAllViews member function in comm_rec is correct as I have it?

 
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 14021012
>>>> persistent->execute(this);    

Actually it would be

   p->execute(this);

;-)


You need to pass 'this' only if execute would need some more information from the comm_rec class, e. g. from which server the message was received. You also could add all additional info you have in comm_rec to a new struct and pass that struct when calling execute . That would be better if you turn to multithreading cause p->execute may run asynchronously in a separate thread and comm_rec already would read next message, thus not being able to answer requests of  a previous message.

Regards, Alex


   
0
 

Author Comment

by:forums_mp
ID: 14022968


I think I'm down to a few confusions on this.   So so far:

   p->execute(this);   // calls polymorphic execute in struct rec_incoming_data
execute calls updateAllViews.

   ptr_comm_rec->updateAllViews(this);    

Is the this pointer here correct?

""""""""""""""  2
Here's another case.  

class test {

private:
   rec_incoming_data *rec;  // (1)
   void some_function();  
   void some_function2();
};

void test::some_function()
{
    if (rec->i == 5)
       // do something
}

void test::some_function2()
{
    if (rec->i == 99)
       // do something
}

void test::receive_data()  // receives all the data from the client.
{
    // later
     data_stream stream(echo_buff, rvcdMsgSize);    

     persistent* ptr_msg = persistent::createInstance(stream);  
     if (ptr_msg)
     {
         persistent->execute(this);
     }      
}

With this approach how would I use 'rec' as is within some_function & some_function2() withouth lots of changes?  IOW:  What do I need to do to within the execute member function to possibly update 'item 1' so that some_function and some_function2 (note: some_function and some_function2 are called independently at different rates) will 'use the latest' from item 1.

struct rec_incoming_data   // incoming message to comm_rec
   : public rec_data
{

  //static bool registered;
  static persistent* create_msg() { return new rec_incoming_data; }

  void execute(test *ptr_test)
  {
      // What happens here that'll allow me to use item 1   ?????  
  }
};
0
 

Author Comment

by:forums_mp
ID: 14025810

Alex, to expand on item 2 above.  On the 'send side'.  I could instantiate the object like so:
 class test {
 private:
   outgoing_msg msg;
public:
 };

and use it anyware

void test::test1()
{
  msg.i = 99;
}

void test::test2()
{
  msg.j = 99;
}

// later I'll store.. and send
void test::send_data()
{
 data_stream* pds = msg.store();
  char *buf(0);
  int  size(0);
  if ((size = pds->getBuffer(buf, 0)) > 0)
  {
    // send buffer
    mSocket->writeBlock(buf, size);  
    writeBlock(buf, len);
  }
}

On the receive side  - similar thing (see my last message above)

0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 14025867
>>>>   ptr_comm_rec->updateAllViews(this);    

>>>> Is the this pointer here correct?

Yes, the 'this' seems correct as it points to the data that should updated in the views. But why is updateAllViews a member of comm_rec? You should separate socket communication classes and visualisation (GUI) classes cause these are different layers. Normally, 'updateAllViews' is a member of an application (model) layer that is between communication and GUI. You could use manager classes that have only one instance as I showed in a previous post.

void rec_incoming_data::execute()
{
    theGuiManager.updateAllViews(this);  
}

>>>> With this approach how would I use 'rec' as is within some_function & 
>>>> some_function2() withouth lots of changes?    

That's a question of architecture. The class 'test' in the sample isn't well defined - or in other words, it isn't clear why you should need a further class 'test' holding a pointer to rec_incoming_data and trying to call test::functions1 or test::function2 dependent on rec_data::i.

Let's try to make it well defined. Assume, rec_data::i is the ID of a client that needs to be informed after the record was displayed in the GUI. If the ID equals 5 the client is on the same computer; if the ID equals 99 it is a remote client. The class test is the manager class that needs to be triggered if a new rec_data has been received. You then have three choices:

(1)
If the manager class is a singleton accessible by a global (extern) declaration, you simply call the appropriate manager function:

// the one and only test manager
extern test theTestMgr;

void rec_incoming_data::execute()
{
      theTestMgr->function(this);
}

where

void test::function(rec_incoming_data* p)
{
     if (p->i == 5)
          function1(p);
     else if (p->i == 99)
          function2(p);
}

(2)
If test is a 'job' class that needs an own instance for any job, you would do that:

void rec_incoming_data::execute()
{
      test job(this);   // create an instance of test
      test.function();
}

where

void test::function()
{
     if (rec->i == 5)
          function1();
     else if (rec->i == 99)
          function2();
}

(3)
If test is a class where the instances already have been created and wait on messages, e. g. by running a message loop, you need to post a message to the message system:

void rec_incoming_data::execute()
{
      Message msg;
      msg.pObject = this;      //  pObject may be of type persistent*    
      msg.sender = 123;       // or any other sender ID
      msg.purpose = 456;     // any message ID
      theMessageMgr->postMessage(msg);
}

where

void test::messageLoop()
{
       Message msg;
      while (getMessage(&msg))
      {
            switch(msg.purpose)
            {
            case ...
                 break;
            case 456:
                 setRecData((rec_incoming_data*)msg.pObject);
                 function();
            ...  
            }
      }
}

Note, the cast isn't necessary if Message struct has a member pointer of type rec_incoming_data* rather than a persistent*. You also may add virtual functions if you need to go back to rec_incoming_data by using a persistent pointer (like execute).

Regards, Alex



0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 14026248
My last post was the answer to your previous comment. I didn't read your last comment before submitting.

>>>> and use it anyware

That might describe the differences in point of view we had sometimes. My goal isn't to supply an interface that could be used *anywhere* but an interface that could be used by the classes which are supposed to use the interface. Generally, the dog should wave it's tail and not vice versa. And a class named 'test' shouldn't be allowed to use anything. I would assume that if you don't find a better name than 'test' you actually don't need the class. It's some kind of OO that makes things worse as you need adapting your good classes in order to make them available for the fake classes.

Regards, Alex



0
 

Author Comment

by:forums_mp
ID: 14028912

Well the use of the name 'test' is certainly bogus.  In general in a forum like this, I often stive to post the minimal amount of code. Often times to do this I illustrate with a bogus name.  In this case 'test'  :)    I'll attempt to use meaningful  names from heneforth since from an OO perspective it's all 'related'.   Some day I'll be as good as Alex and use, in that you could view a problem and think in terms of 'managers' and 'persistents' etc.  etc :)   Currently I struggle with the 'implementation/design' (still going through Modern C++ Designs which while good .. I might need to back up a few .:)) but its imperative that the design makes sense.  In an OO world it no longer, 'just putting code together'.

In any event, I peruse your post again and if I nail this, I'm golden on the 'design' front.

0
 

Author Comment

by:forums_mp
ID: 14029153

Options 1 and 3 are in line with what I'm donig.  The problem with option 1 is 'execute' should not call 'function'.   The member function 'function' is aleady being called by someone else at a select rate.  

That said, I'm down to 3.  I think the answer likes here:

| You also may add virtual functions if you need to go back to rec_incoming_data by using a persistent pointer (like execute).

Caveat:  Well stick with 'test' for now - for discussion purposes.   You see.  After the call to execute and loadFromStream.  The struct rec_incoming_data is already updated with the 'required' stuff.   That said, all I want to be able to do now (within the functions below) is get a copy of rec_incoming_data.  Note the functions (function1 and function2) is rate driven by 'someone' else so there's no need for execute to call them.

void test::function1()
{
   incoming_data in = ............;   // (a)
   if (in.x  == 99)
   {
      // do a special thing.
   }
}

void test::function2()
{
   incoming_data in = ............;  // (b)
   if (in.y  == 200)
   {
      // do a special thing.
   }  
}

The ' ............' highlights the information I'm unsure about.   So how would you accomplish this part.
| You also may add virtual functions if you need to go back to rec_incoming_data by using a persistent pointer (like execute).

One other thing.  It would be nice if I could get the execute (perhaps) member function to return to me the rec_incoming_data since that's really all i need.  The problem I see with the approach above is each function needs to have the lines marked (a) and (b).

I'd prefer something where I could just do:

void test::function1()
{
    if (in.x  == 99)
   {
      // do a special thing.
   }
}

void test::function2()
{
   if (in.y  == 200)
   {
      // do a special thing.
   }  
}

Trouble is.  If 'in' is being updated while I'm reading it then ..
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 14029364
>>>> Some day I'll be as good as Alex

... looking forward ;-)

No, my last comment wasn't meant as an offense, but I was getting tired always to ask if you really need that ...

Note, there are only a few reasons that would justify a  pointer member:

   (1) a pointer of a baseclass that has more than one derived class
   (2) a pointer to the 'parent' class
   (3) a pointer to an implementation class where the original class is a wrapper only

(1) we have if storing a persistent* pointer, say in struct Message from the sample above
(2) we have if a parent class, e. g. a dialog class, passes the 'this' pointer to it's children classes, e. g. controls.
(3) we have if we want to hide the implementation interface of a class. You have a forward declaration of the implementation class in the header of the wrapper class. Cpps and header of the implementation class are not available but a DLL or static library only.

You see, none of these reasons seem to apply to the 'test' class.

Regards, Alex

     
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 14029655
>>>> So how would you accomplish this part.

It's difficult as long as you don't say what the purpose of class 'test'.

Do you need test::function1 and test::function2 be triggered after receiving a record from comm_rec? If yes, why not have function1/function2 members of rec_incoming_data? What's the purpose of class test?

Is it a manager class, where the instance(s) already exist when a rec_data was received or is it a handler/job class that simply covers some special functionality. If first, 'test' is the 'dog' and 'rec_incoming_data' is the 'tail'. You should pass the 'this' pointer (better use a reference) to function1 or function2 (or function).

    theTest->function(*this);

If second, 'rec_incoming_data' might be the 'parent' class and 'test' a (temporary) child class. Then, you would create a new instance of test - passing this (or a reference *this) to the constructor.

    test t_test(*this);
    t_test->function();    // you could call function() in the constructor as well

In any case, you should move the decision whether to call function1 or function2 to class test. Generally, let the objects decide themselves and do not drive one class by another. Simply pass the information needed.

And if you don't know if first or second, you most likely don't need the class ... but that we discussed already ;-)

Regards, Alex
0
 

Author Comment

by:forums_mp
ID: 14031526

Here's the synopsis on how the source works:

void nv_vp::twentyHz_processing()
{
   while (1)
   {
      semTake(one_twenty_sem,  -1);
      if (rec->i == 5) {    // (aa)
         // abc
      }
      else {
        // def
     }
     if ( rec->dd == 99)     // line (bb)
      ///
   }
}

void nv_vp::rx_enet_data()
{
  // this we've been over a million times :)
  // here's where exceute happens.
}

So here's what happens.  I'm awaiting a 'semaphore' from an outside task.   On a 20 Hertz frame, I receive the semaphore.  All I want to do is determine - hence the line marked (aa)  - if  I need to take the path abc or def?

So
| Do you need test::function1 and test::function2 be triggered after receiving a record from comm_rec?

No!!

All I need is some way to update the 'rec' object (lines aa & bb).  This way I get the latest.
So you see calling twentyHz_processing() function doesn't work here.   Once that 'task' is spawned it runs and waits on a semaphore.  This semaphore is triggered at 20 Hertz.

Similarily there's one or two more functions like twentyHz_processing() that sits in a while and wait for a semaphore..  Each 'function/task' has priorities (its a vxWorks thing I'm doing now) hence all that funny context switch etc calls the right guy at the right time .. but the vxworks end of it is irrelevant.

Bottom line.  rec just needs to be updated (which should happen in rx_enet_data) so I could just 'use' it when a semaphore is issued.

0
 
LVL 39

Assisted Solution

by:itsmeandnobodyelse
itsmeandnobodyelse earned 2000 total points
ID: 14034423
>>>> Bottom line.  rec just needs to be updated (which should happen in rx_enet_data)

Some conclusions:

(A)
rec in 'nv_vp' couldn't be identical to persistent pointer created by persistent::createInstance as you pemanently need the 'latest' values. I would suggest to have a rec_incoming_data object (not a pointer) in nv_vp that would be updated by the object received by factory. Note,
you need a mutex/criticalsection when updating the record.

(B)
The nv_vp instance is running in a thread, right? You said, in nv_vp::rx_enet_data() you would call rec_incoming_data::execute (via persistent pointer). But that should be a different thread as you need to wait on socket receive. I don't think it is good if the same instance of nv_vp is running in different threads, or did I unterstand wrongly?

(C)
To update the record in nv_vp you should call an appropriate update function of nv_vp in execute.

   void nv_vp::updateRec(rec_incoming_data& upd)
   {
         enterCriticalSection();     // make update exclusive
         rec = upd;                     // member rec is of type rec_incoming_data
         leaveCriticalSection();    
   }

Note, "rec = upd;" only works if rec_incoming_data has no pointer members. Or if providing an appropriate operator= member function.

Assuming, you have a global instance of nv_vp that could be called from everywhere, you would have

// need
// extern nv_vp theNvVp;
//  
#include "nv_vp.h"      


  void rec_incoming_data::execute()
  {
         theNvVp.updateRec(*this);
  }

Note, by calling execute you don't need a cast. If you don't have a global instance of nv_vp but are calling execute by a member function of nv_vp, you could pass 'this' pointer to execute:

  void rec_incoming_data::execute(nv_vp* pNvVp)
  {
         pNvVp->updateRec(*this);
  }

You might think that it is silly to mutually calling member functions, but it is correct. The execute function virtually makes a branch to the right instance (rec_incoming_data) and *after* that is done, you invoke the update of your parent class. This concept allows to have different derived classes of rec_data later without changing the interface.

Regards, Alex
 
   
0
 

Author Comment

by:forums_mp
ID: 14034986

Agreed with everything said in 'A'.  I'd envision the need for a mutex, hence good point.   I'd hate to be in the middle of updating the 'rec' object and got interrupted by a higher priority task.  The twentyHz_Processing task is one such task.  (i.e higher priority than rx_enet_data).

B).  'ignoring' the semaphore mess rx_enet_data looks like so:

void nv_vp::rx_enet_data()
{
  char echo_buff[4096];
  int rvcdMsgSize(0);

  while (( rvcdMsgSize = socket->receive(echo_buff, RCVBUFSIZE)) > 0)     /// (99)
  {
    data_stream stream(echo_buff, rvcdMsgSize);    
    if (stream)
    {
      persistent *p = persistent::createInstance(stream);  
      if (p != NULL)                                      
      {                
        //p->execute();   // that function does the job     /// (3)
        delete p;                                                  
      }
   }
 }

So (99) handles the socket receive.  

You see in a file called main_nv_vp, I do:
int main()
{
   nv_vp *p = new nv_vp();
}

In the constructor of nv_vp you kick start watchdogs, and all sorts of fancy timers and spawn tasks

nv_vp::nv_vp()
{

  // pseudo code time...
   taskSpawn( rx_enet_data());
   taskSpawn( twentyHz_processing());
// etc
}
To answer your question:  Yes, rec_incoming_data::execute is called via persistent pointer after teh socket receive call (99).  I'm not following the 'thread' issue you alluded to:
| that should be a different thread as you need to wait on socket receive

The receive in the 'thread' (function/thread interchangeable? :)) rx_enet_data
 
C)  I think these two will get me there :)

   void nv_vp::updateRec(rec_incoming_data& upd)
   {
         enterCriticalSection();     // make update exclusive
         rec = upd;                     // member rec is of type rec_incoming_data
         leaveCriticalSection();    
   }

 void rec_incoming_data::execute(nv_vp* pNvVp)
  {
         pNvVp->updateRec(*this);
  }

| You might think that it is silly to mutually calling member functions, but it is correct.
I'm just grateful for the opportunity to learn so much here (learned so much in our previous question and much the same/more here) on how to think OO and structure a design.  Admittidely some things take some 'getting' used to but once I understand why then .. I'm all set.

| Note, "rec = upd;" only works if rec_incoming_data has no pointer members. Or if providing an appropriate operator= member function.
What would the operator= on this look like?

//
Since rec_incoming_data will be initialized statically (which I prefer here).  

  1.  rec_incoming_data rec;
 // later rec_gets updated via:
  2.  rec = upd;                     // member rec is of type rec_incoming_data

Would the create_msg member function below need changing to return a reference instead?

struct rec_incoming_data  
   : public rec_data
{

  //static bool registered;
  static persistent* create_msg() { return new rec_incoming_data; }   /// (aa)
};
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 14037579
>>>> 'ignoring' the semaphore mess rx_enet_data

What do you need the semaphore for? Is it to signal that the socket was ready to receive a message? Or is it a 20 Hz timer?

Did you set the socket to non-blocking?


>>>> void nv_vp::rx_enet_data() runs in a different task

Generally, a class design should guarantee that different threads do not operate with the same class instances. Normally, class instances of different threads communicate by messages or events. If that is too slow, you could have common or global data structs where updates were made exclusivly by using a mutex.

If I understand correctly, your tasks are running static member functions of class nv_vp. Static member functions have no 'this'. However, normally you would pass a pointer to a thread, what might be the pointer of nv_vp - what would be bad as two different threads shouldn't operate with the same class instance - or - much better - the pointer to a shared data struct.

>>>> int main()
>>>> {
>>>>    nv_vp *p = new nv_vp();
>>>> }

The following would do the same but has no memory leaks and compiler warnings...

 int main()
 {
     nv_vp theNvNp;
     return 0;
 }

Note, normally you wouldn't put all functionality of a prog to the constructor but ...

 // make it global
 // then all threads may have access by using an extern declaration
 nv_vp theNvNp();

 int main()
 {
     theNvNp.run();
     return 0;
 }

>>>> I'm not following the 'thread' issue you alluded to:

Simply means that you should use a separate thread when reading from a socket (apparently you did so).

There are 3 kinds of techniques to read from a socket:

  1. Read and block (wait on recv call) if nothing was sent.

    Problem: You have to kill the thread or send a finish message if you want to exit the program.

  2. Peek for message by calling select with timeout. Read if message exist and poll if not.

    That is the recommended (but not easiest)   way.

  3. Set socket to non-blocking and poll for receive.

  You would get SOCKET_ERROR at most reads if there is no message available.

 
>>>> What would the operator= on this look like?

  struct rec_data
  {
        int i;
        char c;
        string s;
        char* p;

        rec_data& operator=(const rec_data& rd)
        {
              i = rd.i;
              c = rd.c;
              s = rd.s;
              p = NULL;
              if (rd.p != NULL)
              {
                  p = new char[strlen(rd.p)+1];
                  strcpy(p, rd.p);
              }
              return *this;
        }
  };

Note, you need an operator= () only because of the pointer. All other members would be copied correctly wwith a default operator=, the compiler automatically would create if none was supplied. That's one of the reasons why you shouldn't use pointers...

>>>> return *this;

That is because of ....

    rec_data  r1, r2, r3, r4;

    r4.i = 1;
    r4.c = 'A';
    r4.s = "Hello";
    r4.p = NULL;

    r1 = r2 = r3 = r4;   // cascading assignments by using *this return

>>>> Would the create_msg member function below need changing to return a reference instead?

No, for two reasons:

1. On fail, it has to return a NULL pointer; but there is no NULL reference in C++.

You could overcome that by having a static *nil* instance where you would return a reference to if the create fails. However, that is much efforts and overkill for a small advantage...

2. You cannot store references in a array. So you would need to make copies - but you can't make a copy of an abstract baseclass -  or transform back to a pointer what make things worse...

Regards, Alex
 
0
 

Author Comment

by:forums_mp
ID: 14040775

| Did you set the socket to non-blocking?
I just performed a few tests and realized yeah you're right.  This thing is blocking.  That's a bummer :).  Quite frankly I was so consumed with persistent :) that I lost track of everything else.   Of course I haven't merged the socket code into my application but here's what I've been using in my tests:

http://cs.ecs.baylor.edu/%7Edonahoo/practical/CSockets/practical/TCPEchoServer.cpp
http://cs.ecs.baylor.edu/%7Edonahoo/practical/CSockets/practical/PracticalSocket.cpp
http://cs.ecs.baylor.edu/%7Edonahoo/practical/CSockets/practical/PracticalSocket.h

accept and recv (the header file doesn't state that but recv does block) blocks.  Thats not good. :(

Good catch!!  Now I have to rethink this.  From the looks of the source I can't seem to see anyway to turn off the blocking.  Bummer :(

I think option 3 in your post is the best approach since if I were to consider three functions.

void nv_vp::function1()
{
}
void nv_vp::function2()
{
}
void nv_vp::function3()
{
}

void nv_vp::rx_enet_data()
{
}

Now all four functions are spawned with 'priorities' assigned.  rx_enet_data has the lowest priority.  Based on the source code resident at those links.  My theory is as follows: (correct me if I'm wrong)  Assume the tasks are spawned in the order outlined.  When rx_enet_data gets spawned and it blocks the CPU is consumed by rx_enet_data until a connection is established.  It blocks again on recv function1.  That said function1, function2 and function3 will - for the most part - never see the CPU.  

Option 2 in your post seems viable but I cant even figure out how to fit that in.  Time for a new class - I think.  :)
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 14043681
>>>> When rx_enet_data gets spawned and it blocks the CPU is consumed by rx_enet_data until a connection is established.

No, *blocking* is another word for *waiting*. While being blocked the thread doesn't consume any CPU time. On the contrary, the task scheduler of the operation system
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 14043899
... wasn't finished but the browser crashed @!*; ... all text gone

... contrary, the task scheduler of the operation system preferable uses IO operations or function calls to switch to another thread...

>>>> This thing is blocking

Do you mean waiting (A) or running infiitely consuming nearly all CPU (B)

The official meaning of blocking is (A) that recv or receive is waiting for the next message. You couldn't activate or terminate the thread beside of sending a message from a client or by killing the thread (what isn't recommendable). To avoid that, you either could set the socket to non-blocking

    unsigned long flag = 1;
    ioctlsocket(sock, FIONBIO, &flag);

or call select with timeout to check for messages *before* calling receive/recv.

If (B) your socket already is set to non-blocking. Then, you need to slow down your infinite loop by short waits - say 10 milliseconds.

   while (recv(sock, ...) == SOCKET_ERROR && GetLastError() == WSAEWOULDBLOCK)
   {
         // good chance to check a global exit flag
         // that could be set by the main or dialog thread
           
         if (g_timeToExit)
              return;           // stop thread

         Sleep(10);   // wait a little time to give other threads some of cpu
     }

Note, error function/error code and sleep function may be different on non-Windows platforms.

The select function is more portable but needs more code. Tell me, if you need a sample.

Regards, Alex
0
 

Author Comment

by:forums_mp
ID: 14046063

| No, *blocking* is another word for *waiting*. While being blocked the thread doesn't consume any CPU time.
| On the contrary, the task scheduler of the operation system preferable uses IO operations
| or function calls to switch to another thread

I see.  That means then that if a semaphore is issued to - say function1, function2 or function3(my previous post).  Execution continues within those functions and is independent of the 'waiting' within the rx_enet_data function.  Correct!!

| Do you mean waiting (A) or running infiitely consuming nearly all CPU (B)
A is correct.   I misunderstood the meaning.

| The select function is more portable but needs more code. Tell me, if you need a sample.
I could always use an Alex sample :)  Note though on the windows side (my client) I'm using trolltech's 'QT' APIs and those work well.  The issue is on the 'server' side (vxWorks/GCC).  That said GetLastError and sleep wont work.  Let me know!!
0
 

Author Comment

by:forums_mp
ID: 14046253
Bye the way, Alex, have you worked with QT before?
0
 
LVL 39

Assisted Solution

by:itsmeandnobodyelse
itsmeandnobodyelse earned 2000 total points
ID: 14046528
>>>> Bye the way, Alex, have you worked with QT before?

No, I am sorry.

>>>> I could always use an Alex sample

enum { NET_SUCCESS, NET_TIMEOUT, NET_SELECT_FAILURE };

/*------------------------------------------------------------------------------*/
/* selectRead is a helper routine to peek for incoming connections or messages  */
/* on a socket that is in listening or connecting state.                        */
/* A timeout value can be set to prevent blocking.                              */
/* Possible return codes are NET_SUCCESS, NET_TIMEOUT or NET_SELECT_ERROR.      */
unsigned long selectRead(socket sockHost, int msecWait)
{
    int     result = 0;

    /* fd_set manages an array of sockets */
    fd_set          readSockets;
    struct timeval  tval;

    /* set host socket as first and only element */
    FD_ZERO(&readSockets);
    FD_SET(sockHost, &readSockets);

     /* init timeout */
    tval.tv_sec  = msecWait / 1000;
    tval.tv_usec = (msecWait % 1000) * 1000;

    result = select(sockHost + 1, &readSockets, NULL, NULL, &tval);

    if (result == 0)
        return NET_TIMEOUT;
    else if (result == SOCKET_ERROR)
        return NET_SELECT_FAILURE;

    return NET_SUCCESS;
}

That could be used like

    ...
    /* loop will not end until external cancel flag is set or an error occurs   */
    while (true)
    {
        /* check socket for incoming data with timeout */
        result = selectRead(sockClient, 10);
        if (result == NET_SUCCESS)
        {
              // now recv won't block
              result = recv(sockClient, ...);
        }
        else if (result == NET_TIMEOUT)
        {
               // check global exit flag
               if (g_timeToLeave)
                    return;
               continue;   // continue loop; we don't need sleeping cause already waited
        }
        // socket error
        return;
    }

>>>> That said GetLastError and sleep wont work.  Let me know!!

In unix it is errno and usleep. However I don't know exactly the corresponding error code of WSAEWOULDBLOCK. Maybe it is EWOULDBLOCK.


#include <errno.h>
    ...
    while (recv(sock, ...) == SOCKET_ERROR && errno == EWOULDBLOCK)
    {
          usleep(10000);   // as far as I know it is microseconds  
    }  

Regards, Alex
0
 

Author Comment

by:forums_mp
ID: 14048507

Just so I get one thing straight on this blocking/non-blocking approach.  Answer this for me:

You wrote:

| No, *blocking* is another word for *waiting*. While being blocked the thread doesn't consume any CPU time.
| On the contrary, the task scheduler of the operation system preferable uses IO operations
| or function calls to switch to another thread

I wrote:
I see.  That means then that if a semaphore is issued to - say function1, function2 or function3(my previous post).  Execution continues within those functions and is independent of the 'waiting' within the rx_enet_data function.  Correct!!

Is my assessment correct?

|    while (recv(sock, ...) == SOCKET_ERROR && errno == EWOULDBLOCK)
|    {
|          usleep(10000);   // as far as I know it is microseconds  
|    }  
So it's quite possible to get a non-blocking approach by going this route?

| errno == EWOULDBLOCK
But who's changing/where does errno get updated?
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 14048755
>>>> if a semaphore is issued to - say function1, function2 or function3(my previous post).  
>>>> Execution continues within those functions and is independent of the 'waiting' within
>>>> the rx_enet_data function.  Correct!!

Generally, threads are invoked independently beside of the case they explicitly wait on another, e. g. when using a mutex.  I didn't answer the question cause I don't know whether each of your threads has it's own semaphore or if there is one that is shared among threads. Anyway, the socket thread is independent. While waiting to receive messages there is little to none influence to other threads. Of course taht is different if you would remove the sleep or wait functions. In that case, the thread would consume nearly all CPU and all other threads would slow down because of that.

>>>> So it's quite possible to get a non-blocking approach by going this route?

Yes, but using select is the better (and faster) approach. It's faster cause the timeout wait in select would break when a message arrives while the sleeps always will last 10 milliseconds even when a message arrived in the meantime.

>>>> But who's changing/where does errno get updated?

errno is a global error variable, that was set by recv/receive function when an error occurs. The problem with errno is that it isn't thread-safe, i. e. if a second thread calls functions setting/using errno nearly same time you might get the wrong value from another thread.

Regards, Alex

 
0
 

Author Comment

by:forums_mp
ID: 14050449

Alex, we're getting close to a baseline on basic_stream and I've got to tell you, 'We' can't thank you enough.  A while back I mentioned the use of typeid to you.   My intent is to use typeid to determine the primitive types, more specifically int, double, float, longs.  These types need endian conversion.  Here's what currently works.

#define ByteSwap5(x) ByteSwap((unsigned char*)&x, sizeof(x))

static void ByteSwap(unsigned char *b, int num)
{
  register int idx(0);
  register int jdx(num - 1);
  while (idx < jdx)
  {
    std::swap(b[idx], b[jdx]);
    idx++, jdx--;
  }
}

// only done for ints and doubles right now:
template <class T>
inline data_stream& operator<<(data_stream& ds, const T& t)
{
  if (typeid(t) == typeid(int))
   ByteSwap((unsigned char*)&t, sizeof(t));

  if (typeid(t) == typeid(double))
   ByteSwap((unsigned char*)&t, sizeof(t));

  if (ds.isValid())
  {
    int curPos   = ds.curPos;
    ds.curPos += sizeof(T);
    ds.buffer.resize(ds.curPos);
    memcpy(&ds.buffer[curPos], &t, sizeof(t));
  }
  return ds;
}

Do you see any issues with this approach?   Do you see a way we could achieve the same thing with a reference?  The fact that the address of t is passed to ByteSwap makes this (I think) ok  but I'd prefer a reference.

We also think it's better to have the conversion on the the server side.   Your thoughts:

An aside:
I'd like to continue - what I presume from here - will be a set of 'miscellaneous' questions.  The design on data_stream for the most part has been solidified and as such questions the miscelleanous pile will not be 'design related'.  In any event, the problem with experts exchange is I can't create a question for 'a' particular expert.  You have to 'put it out there' but.. I'll work with community service on that.

Your 05/05/2005 06:31AM PDT post.  More specifically transferbuffer.  I don't fully understand the changes required to use transfer buffer.

A few questions with regard to an implementation surrounding the checksum parameter of the message header and a question on your 05/04/2005 12:41PM PDT post.
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 14058592
>>>> Do you see any issues with this approach?  

I personally never used RTTI. Seems somehow foreign to C++. Asking for typeid instead of using polymorhism doesn't seem an elegant approach. So, I would prefer to use htonl/htons or have a system-dependent byteswap function - like the one you posted - to convert to a known format (the function would do nothing if endianess is correct).

>>>> but I'd prefer a reference.

No problem when using polymorphism instead of char array

union Plain2UChar
{
     int i;
     unsigned int ui;
     long l;
     unsigned long ul;
     short s;
     unsigned short us;
     double d;
     unsigned char uc[8];
     
     void byteSwap(int siz)
     {
          int idx = 0;
          int jdx = siz-1;
          while (idx < jdx)
          {
                std::swap(uc[idx], uc[jdx]);
                idx++, jdx--;
          }
     }
     Plain2UChar(int ii) : i(ii)    { swap(i); }
     Plain2UChar(double dd) : d(dd) { swap(d); }
     unsigned char* getBuf()  { return uc; }

     static void swap(int& i)
     {
         Plain2UChar x = i;
         x.byteSwap(sizeof(i));
         i = x.i;
     }
     static void swap(double& d)
     {
         Plain2UChar x = d;
         x.byteSwap(sizeof(d));
         d = x.d;
     }
   
};

Could used like

int main()
{
     // if you need plain data swapped
     double d  = 12345.678;
     Plain2UChar::swap(d);
     int i  = 12345;
     Plain2UChar::swap(i);

     // if you need char buffer
     Plain2UChar iuc(12345);
     unsigned char* bufi = iuc.getBuf();
     Plain2UChar duc(12345.678);
     unsigned char* bufd = duc.getBuf();
     
      return 0;
 }


>>>> Your 05/05/2005 06:31AM PDT post.  More specifically transferbuffer.  

If a function has to return more than one return value, it is good practice to define a struct containing all return arguments as data member and return values by returning a pointer or reference to that struct. Normally, you return a reference to the struct given as an argument (similiar to operator<< overloads).

   MyStruct& func(MyStruct& ms);


>>>> the problem with experts exchange is I can't create a question for 'a' particular expert

No problem. Simply, give me a link to the new question here and give a link to this question in the other. If other experts want to participate they could. However, it might need some efforts to follow this thread that has some million of lines, I supoose ... ;-)


Regards, Alex
0

Featured Post

Technology Partners: We Want Your Opinion!

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

C++ Properties One feature missing from standard C++ that you will find in many other Object Oriented Programming languages is something called a Property (http://www.experts-exchange.com/Programming/Languages/CPP/A_3912-Object-Properties-in-C.ht…
Jaspersoft Studio is a plugin for Eclipse that lets you create reports from a datasource.  In this article, we'll go over creating a report from a default template and setting up a datasource that connects to your database.
The goal of the tutorial is to teach the user how to use functions in C++. The video will cover how to define functions, how to call functions and how to create functions prototypes. Microsoft Visual C++ 2010 Express will be used as a text editor an…
The viewer will be introduced to the member functions push_back and pop_back of the vector class. The video will teach the difference between the two as well as how to use each one along with its functionality.

834 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question