Start Free Trial

asked on

serialization technique.

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;
};

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?

ASKER

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.

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.

ASKER CERTIFIED SOLUTION

itsmeandnobodyelse

membership

This solution is only available to members.

To access this solution, you must be a member of Experts Exchange.

Start Free Trial

ASKER

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.

itsmeandnobodyelse

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

ASKER

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?

itsmeandnobodyelse

>>>> 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

ASKER

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;

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.

ASKER

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.

itsmeandnobodyelse

>>>> 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

ASKER

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;

itsmeandnobodyelse

>>>> 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

SOLUTION

itsmeandnobodyelse

membership

This solution is only available to members.

To access this solution, you must be a member of Experts Exchange.

Start Free Trial

itsmeandnobodyelse

Have to leave now. Would return tomorrow (after 11 hours).

Regards

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/

ASKER

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();
}

SOLUTION

itsmeandnobodyelse

membership

This solution is only available to members.

To access this solution, you must be a member of Experts Exchange.

Start Free Trial

ASKER

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

ASKER

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
};

itsmeandnobodyelse

>>>> 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

ASKER

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?

itsmeandnobodyelse

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

ASKER

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.?

ASKER

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.

itsmeandnobodyelse

>>>> 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

ASKER

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
}

itsmeandnobodyelse

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

ASKER

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?

SOLUTION

itsmeandnobodyelse

membership

This solution is only available to members.

To access this solution, you must be a member of Experts Exchange.

Start Free Trial

ASKER

>>> 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 :)

ASKER

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.

ASKER

Alex, I think I figured it out.

itsmeandnobodyelse

>>>> 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

ASKER

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;

itsmeandnobodyelse

>>>> 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

ASKER

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?

ASKER

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?

SOLUTION

itsmeandnobodyelse

membership

This solution is only available to members.

To access this solution, you must be a member of Experts Exchange.

Start Free Trial

ASKER

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?

SOLUTION

itsmeandnobodyelse

membership

This solution is only available to members.

To access this solution, you must be a member of Experts Exchange.

Start Free Trial

ASKER

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 :)

ASKER

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.

ASKER

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?

ASKER

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.

SOLUTION

itsmeandnobodyelse

membership

This solution is only available to members.

To access this solution, you must be a member of Experts Exchange.

Start Free Trial

itsmeandnobodyelse

>>>> 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

ASKER

>>>> 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

itsmeandnobodyelse

>>>> 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

ASKER

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.

SOLUTION

itsmeandnobodyelse

membership

This solution is only available to members.

To access this solution, you must be a member of Experts Exchange.

Start Free Trial

ASKER

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.

ASKER

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.

itsmeandnobodyelse

>>>> 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

ASKER

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.
.

SOLUTION

itsmeandnobodyelse

membership

This solution is only available to members.

To access this solution, you must be a member of Experts Exchange.

Start Free Trial

ASKER

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

itsmeandnobodyelse

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

ASKER

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

itsmeandnobodyelse

(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

ASKER

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)
}

itsmeandnobodyelse

>>>> 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

ASKER

[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?

itsmeandnobodyelse

>>>> 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

ASKER

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;
}
}

itsmeandnobodyelse

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

ASKER

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!!

ASKER

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.

itsmeandnobodyelse

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

ASKER

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;
};

ASKER

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.

itsmeandnobodyelse

>>>> 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

ASKER

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.

itsmeandnobodyelse

>>>> 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

ASKER

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.

itsmeandnobodyelse

>>>> ... 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

ASKER

| 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?

itsmeandnobodyelse

>>>> 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

ASKER

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 ?????
}
};

ASKER

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)

itsmeandnobodyelse

>>>> 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

itsmeandnobodyelse

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

ASKER

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.

ASKER

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 ..

itsmeandnobodyelse

>>>> 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

itsmeandnobodyelse

>>>> 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

ASKER

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.

SOLUTION

itsmeandnobodyelse

membership

This solution is only available to members.

To access this solution, you must be a member of Experts Exchange.

Start Free Trial

ASKER

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)
};

itsmeandnobodyelse

>>>> '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

ASKER

| 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. :)

itsmeandnobodyelse

>>>> 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

itsmeandnobodyelse

... 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

ASKER

| 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!!

ASKER

Bye the way, Alex, have you worked with QT before?

SOLUTION

itsmeandnobodyelse

membership

This solution is only available to members.

To access this solution, you must be a member of Experts Exchange.

Start Free Trial

ASKER

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?

itsmeandnobodyelse

>>>> 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

ASKER

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.

itsmeandnobodyelse

>>>> 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

ASKER

Alex, refer to this link:

https://www.experts-exchange.com/questions/21435097/Part-II-continuation-of-a-serialization-technique-question.html