Link to home
Start Free TrialLog in
Avatar of forums_mp
forums_mp

asked on

Part II / continuation of a serialization technique question


Alex,

Referencing - our initial question:
https://www.experts-exchange.com/questions/21402129/serialization-technique.html

Now consider:

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

How would I get around the fact that the persistent class' execute function now needs to be 'specialized'?  

IOW:
class persistent
{
  // factory map is mapping class names to create functions
     static std::map<std::string, CreateInstanceFunc> factoryMap;
public:
  virtual void execute (  nv_vp* pNvVp ) = 0;   // (1)
};

Would like not to do item marked 1.   That said this would keep persistent 'pure'.  If not then I'd have an execute member function for each data type which I'm not a fan of.
virtual void execute (  nv_vp* pNvVp ) = 0;   // (1)
virtual void execute (  ts_vp* pTsVp )  = 0;   // (2)
// and so on..

In addition.  In order for the ' // (a)' to work.  The pointer obviously needs to be initialized, however I'll pass in a initialized pointer.  Microsoft issues a complaint about initilization.

Alternatively.  I suspect I need to peruse the use of static instance. i.e

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

However, I'd like to try to find a solution to the pointer approach prior to using the static instance approach.
Avatar of itsmeandnobodyelse
itsmeandnobodyelse
Flag of Germany image

>>>> How would I get around the fact that the persistent class' execute function now needs to be 'specialized'?  

You would need a manager class that is superior (or parent) both of nv_vp and ts_vp, say vp_mgr.

class vp_mgr
{
     nv_vp  theNvVp;
     ts_vp   theTsVp;
public:
     nv_vp& getNvVp { return theNvVp; }
     ts_vp&  getTsVp { return theTsVp;  }
   
};

Then persistent::execute either has a reference to vp_mgr or you have a global 'theVpMgr' where all classes have access to. The overload of rec_incoming_data::execute turns to

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

An alternative to that would be to derive both nv_vp and ts_vp from vp_mgr and pass a vp_mgr pointer to execute. Then, updateRec must be a pure virtual function of vp_mgr, overloaded both by nv_vp and ts_vp. However, as we have singletons, I would prefer the first solution.

Regards, Alex


Avatar of forums_mp
forums_mp

ASKER


Alex, re-structuring the code to use static initilization is enough to drive me crazy.

// commrecorder.h
#ifndef  COMM_REC_H
#define COMM_REC_H

struct sndr_to_rcvr_class     // note name here are for illustration
  : public persistent
{
  int             i;
  double       d;
  std::string  s;
  bool           b;
  char           c;    
 // more
};

struct rcvr_to_sndr_class    // note name here are for illustration
  : public persistent
{
  int              i;
  double       d;
  std::string  s;
  bool           b;
  char           c;    
  // more
};

struct rcvr_to_sender                      // note name here are for illustration
  : public rcvr_to_sndr_class
{
  static persistent* create_msg () {
    return new rcvr_to_sender;
  }
  void execute() {
    mComm.updateRec(*this);   /// (99)
  }
};


class QSocket;
class CommRecorder : public QObject
{
  Q_OBJECT
public:
  CommRecorder();
  ~CommRecorder();
private:
  sndr_to_rcvr_class  outgoing;   // instantiate
  rcvr_to_sender       incoming;   // instantiate .. this might be bad
};

#endif

// rec_win_impl.h
#ifndef   REC_WIN_IMPL_H
#define  REC_WIN_IMPL_H

class CommRecorder;  // forward declaration of CommRecorder.

class RecorderWinImpl : public RecorderWin
{
  Q_OBJECT
  void slotReaded();
public:
  RecorderWinImpl(QWidget *parent = 0L);
  ~RecorderWinImpl();

private:
   ///CommRecorder *mComm;
};

#endif

// rec_win_impl.cpp
#include "commrecorder.h"
RecorderWinImpl::RecorderWinImpl(QWidget *parent)
 : RecorderWin(parent)
{
  ///mComm = new CommRecorder();
  ///connect(mComm, SIGNAL(connected()), this, SLOT(slotConnected()));
}

The commented out instance of mComm is what I currently have that worked.  The implementation file (RecorderWinImpl) handled construction/destruction of the CommRecorder object.
Now I need to taylor this to use the static intilization approach so that '99' could play nice.  

/////////////////////////////////////////////////////////////////////////////////////////////////////////////////
Similarily on another platform I have:

//nv_vp header
#ifndef   NV_VP_H
#define  NV_VP_H
class nv_vp {
public:
  nv_vp ();
  void updateRec ( rcvr_to_sender& data);
};
#endif

// nv_vp.cpp
nv_vp::nv_vp() {}

// in main_nv_vp
// nv_vp * ptr_nv_vp(0);
int main()
{
  // ptr_nv_vp = new nv_vp();
}

Now for the interesting part:  In a file called
//msg_interface.h
struct rcvr_to_sndr_class  // note name here are for illustration
  : public persistent
{
};

struct rcvr_to_sender      // note name here are for illustration
  : public rcvr_to_sndr_class
{
  static persistent* create_msg () {
    return new rcvr_to_sender;
  }
  void execute() {
    nv_vp.updateRec(*this);   /// (100)
  }
};

This one appears easier but same story.   How would you modify the commented out object ptr_nv_vp to use static initlization such that  100s to play nice.  If memory serves me well, here's what I tried with zero success.

// nv_vp.cpp
nv_vp::nv_vp() {}
void nv_vp::updateRec(rcvr_to_sender& data)
{
}

//msg_interface.h
extern nv_vp  nv_vp_;       // extern here...........
struct rcvr_to_sndr_class  
  : public persistent
{
};

// in main_nv_vp              
int main()
{
  nv_vp  nv_vp_;                 // now we're ready
}

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

template <class T>
inline data_stream& operator<< (data_stream& ds, const string& s) {}    // (44)

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

template <class T>
inline data_stream& operator<< (data_stream& ds, bool& b) {}             // (66)

Notice the insertion operators are defined for constant objects, except the latter which is defined for non-constant object.  For a minute I thought I needed two more for non-constant string and 't'.  So I added.

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

Prior to the addition - of the line (55) above -  on the client side, I'd send  
99,         (decimal)
9.0,        (double)
testing,   (string)
0,           (bool)
'A'.         (char).

The server would receive the data as expected.  During a debug session, I'd even observe the insertion operation on string being called.  IOW, I could step into/through 44.

When I add 55, 44 is skipped and the receiver now receives:

99,         (decimal)
9.0,        (double)
IIItesting,   (string)
1,           (bool)
I.         (char).

Notice the 'III' before testing and the I in the character field.   III' is a set of funny characters with what's akin to a hyphen above the 'I'.

For starters, I thought I needed const and non-const version of both operators in a case like this?  If not why?
Now the big question:  :) In theory what I should have is const's for operator << and non-constants for operator >>?  That said (66) need not be there?  Also whats the issue with the 'string' why things went amuck?  My guess is it's becasue 'string' is theoretically a constant?  If thats true though I'd expect the const version of string to be called.  Again No version of string (<< (data_stream& ds, const string& s)) was called when I added 55.

One other thing.  Something is amiss about the extraction versions of this:
inline data_stream& operator>> (data_stream& ds, bool& b)
{
   if (ds.isValid())
   {
     unsigned char uc;
     ds >> uc;                                // (aa)
     b == (uc != '\0') ? true : false;  // (bb)
   }
   return ds;
}

For (aa).  The contents of the stream are placed into uc.
(dd).    uc is checked against null.   b gets assigned true or false.   ds is returned?  What's the point of the check? b is a reference but I dont quite get the return on ds..

The insertion operator makes more sense to me.

inline data_stream& operator<< (data_stream& ds, const bool& b)
{
   if (ds.isValid())
   {
     unsigned char uc = b ? 0x01 : 0x00;
     ds << uc;                            
   }
   return ds;
}
The contents of uc is written to ds and return... make sense :)






 
SOLUTION
Avatar of itsmeandnobodyelse
itsmeandnobodyelse
Flag of Germany image

Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
| I don't know why you think you need two identical data classes. The point is that the data part of classes to serialize is identical and the
| functional part could be different. That is done by deriving a sender class and a receiver class both from the *same* data base class. Of
| course, it also works with two identical data classes, but I always get doubts whether you fully understood the concept...

Alex, the identical classes are only for illustration.   My real application doesn't have identical outgoing and incoming messages.   My intent is to achieve success  - which we have except the nuianse execute function :) - with a 'generic' test case.  Having said that, object names such as
int i, double d, string s,  etc.  - believe me - aren't the real names in my application.

| Also, static initialization of data members couldn't rely on the order of other static data member initialization. Therefore you need to minimize
| static member dependencies. Again, separate creation and functionality. In main(), you can do all one-time-initializations either using a
| manager (class) or by calling static initialization/registering functions directly. However, that only works if you remove functionality from the
| constructors where you are dependent on other statics already have been initialized.

I need to rethink all the free store initilization I've been doing.  I also need to get my head completely around this static initlization/initilization order stuff.  I need sound solutions that I could keep in my back pocket for future use.    I wish I could do attachments here.  It would make like so much easier.  Anyways..

///
I understand the
<conditional_expression> ?  <value_expression> : <value_expression>
stuff :)

What I wasn't seeing is the return of ds here.

inline data_stream& operator>> (data_stream& ds, bool& b)
{
   if (ds.isValid())
   {
     unsigned char uc;
     ds >> uc;                                // (aa)
     b =  (uc != '\0') ? true : false;  // (bb)
   }
   return ds;
}
The contents of the stream gets put in uc.  b changed depending on the conditional expression.   ds got returned but ds does not 'have' the update made to b?   Come to think of it, b is a reference so any changed made to b should be reflected in ds.
SOLUTION
Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
| Still worried.

Here's a sample definition.  Lets assume two devices:   Call them A (client) and B (server).
Client sends:
    MODE            mode;
    unsigned int    on_off_time;
    unsigned int    update rate;

Server sends
   MODE_STATUS  mode;
   unsigned int       on_off_time_status;
   unsigned int       status_count_a;
   unsigned int       identification_a;
   unsigned int       status_count_b;
   unsigned int       identification_b;  

For the purposes of discussion MODE and MODE_STATUS are enumerations.   So now the client sends the three items listed.  The server sends the six items in response.   So on the client & server side we'll need 'both' lists.   That's where we're headed ultimately.
The fact that the data in 'my' earlier code is idential is ONLY for test.  I'm only trying to prove a concept.  IOW: I'm happy with the way things work now.   Rolling in 'my' real data becomes a breeze.

As far as I see it.  There' three sticking points:
1.   Fully understanding what this means when using bit fields.
2.   Having an execute member function in both sender and receiver data structs - on the client and server side - is somewhat disturbing and I'd really like to rid that.  Note the issue here is one execute member function is meaningless but needs to be there because of the need to rid the dynamic cast  and use persistent directly.
3.   The execute member function and the need for static initilization of objects.   This allows one to call an updateRec member function in the 'parent' (is that the right word) class.  


///////
What do you do now when you had a try catch handler prior?   So now:

nv_vp  *nv_vp_(0);                 // now we're ready
int main()
{
  try {
    nv_vp_ = new nv_vp();
  }
  catch ( nv_vp_error& err)
  {
    err.what();
  }
}

Now I have to figure out, how to re-tailor the class and the exceptions etc and that's where it get hairy.

ASKER CERTIFIED SOLUTION
Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial

| Bitfields are used for three reasons:
Reason 4 :)
I have a requirements document where the message is defined as such - bit fields (my first post in our previous thread shows an example of such struct).   I'm off challenging it but I haven't succeed yet :)
In the interim, I think worse case.  If I have to stick with the bit fields what does it mean to data_stream.   This is by far the 'most' frightening issue with respect to one 'measly' piece I'm working with.   data_stream of course - is the route I'm going on 'other stuff'.  One of the key things i like about the stream approach is it eliminates the need for the message header which contains msg_id, msg_count, msg_size and checksum.   So now:  msg_id  per data stream - 'already' defined.  msg_count - per data_stream - ok I could hard code this in.   msg_size - per data_stream 'theoretically' already defined.  checksum - get rid of this.  


| Z. Have two persistent derived classes out_persistent and in_persistent and move pure virtual execute to in_persistent class.
I'm assuming that with this option.  
class persistent will be akin to:

class persistent
{
  // factory map is mapping class names to create functions
  static std::map<std::string, CreateInstanceFunc> factoryMap;
public:
  virtual std::string    getClassId() = 0;
  static persistent* createInstance(const stream& ds)
  { }

  static bool registerClass(
            const std::string& className,
            CreateInstanceFunc createFunc
        ) {}

   // data_stream *store() {} // (66)
};

The line 66 could be moved to 'out_persistent'.   I'll be gone for a few days so I'll look into this.   I'll keep this solution in my 'backpocket'.  I'm trying to learn/get all what's cool and the alternatives - just in case :)

|  class send_recv_message : virtual public out_persistent,
                              virtual public in_persistent

'virtual'?  I'll get back to you on all this 'virtual' in a inheritance setup.  First I need to do a refresher.

| I told you - more than once - how to solve this: you need to move the registerClass calls to main or to a manager class called by main.
| That assures that static persistent::factoryMap was instantiated *prior* to the first call that needs the factoryMap.

Right!!  I'm not referring to factoryMap.  I'm referring to:

nv_vp nv_vp_;               // (1)
int main()
{
}

// then I'll need this.
extern nv_vp nv_vp_;      // (4)

In theory no big deal but I think I've been skewed towards the dynamic allocation.    I think there's no way around 4 (pointer or non pointer) hence doing 1 should be ok.

| Actually, I don't like exceptions
Oh no!!!  :)  I see your points but more on your last paragraph later.


Alex consider your comment on an issue with address aritmetic.

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

I'd like to revist - more specifically - the wapper portion you alluded to.  So now the basics:
The impetus:  A DMA operation.   To perform the DMA operation DMA 'records' are created.  Max size is 15 MiB.   That said, if the user desires to DMA 32 MiB of data.  Two records will be created.  Max number of records is 10.

Helpful member functions & structs:

void * cacheDmaMalloc
    (
    size_t bytes /* number of bytes to allocate */
    )

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

Here's my approach.  User will pass in soruce/destination address and size.

  dma_xfer(
    unsigned int    *source_addr,
    unsigned int    *dest_addr,
    unsigned int     xfer_size )    
 {
    unsigned int     tx_bytes(0);
    RECORD         *rec_chain;

    bool success(true);

    unsigned int MebiByte(15 * 1024 * 1024);  // max size
    int num_records(0);
    const unsigned int MAX_TRANSFER_COUNT(MebiByte);

    if (xfer_size > MAX_TRANSFER_COUNT )                                   // if > 15 MiB determine number of records needed
      num_records = (xfer_size / MAX_TRANSFER_COUNT ) + 1;    
    else
      num_records = 1;

      rec_chain = (RECORD*)cacheDmaMalloc(
                                 sizeof (RECORD) *
                                 num_records
                               );                                       // allocate a record chain
      if (rec_chain)                                               // if success setup the rec_chain struct
      {
        for (int idx(0), jdx(1); idx < num_records; ++jdx, ++idx)
        {
          if (xfer_size > MAX_TRANSFER_COUNT)  {
            tx_bytes = MAX_TRANSFER_COUNT;
            xfer_size  -= MAX_TRANSFER_COUNT;

            rec_chain[idx].NextRecPtr  = (unsigned int)&rec_chain[jdx];
          }
          else {
            tx_bytes = xfer_size;
             rec_chain[idx].NextRecPtr = 0;
 
          }
 
          rec_chain[idx].ByteCnt    = tx_bytes;
          rec_chain[idx].DestAdd    =
               (unsigned int)dest_addr   + (jdx * tx_bytes);
          rec_chain[idx].SrcAdd     =
              (unsigned int)source_addr  + (jdx * tx_bytes);
       }
     
#if 0   // now do the DMA.  The last parameter is the the rec_chain struct
        STATUS status =  DmaTransfer2 (
                                     ENG_0,
                                    (unsigned int)source_addr,
                                    (unsigned int)dest_addr,
                                    xfer_size,
                                    &rec_chain[0]             // give the chain to the DMA engine.  It'll take care of things
                                  );
#endif
         // validation

      }
  }

Given the requirements, how would you re-write this per your recommendations?
Alex, you there? :)
>>>> Alex, you there? :)

Yeah, but still have problems to understand why you should need that.

>>>> To perform the DMA operation DMA 'records' are created.  Max size is 15 MiB

Why should they have such an odd limitation? Isn't that a simple constant that you could overwrite?

e. g.

#define MAX_DMA_RECORD_SIZE 32 * 1024 * 1024
#include <xfer.h>    // include header after defining a different maximum

>>>> That said, if the user desires to DMA 32 MiB of data.  Two records will be created.

Why not simply allocate two buffers by two calls? Why do you think you need to bother with offsets and record index? Why do you think you need 32 megabytes?

>>>>      rec_chain = (RECORD*)cacheDmaMalloc( sizeof (RECORD) *  num_records );

Actually, that statement seems to be wrong. If num_records == 2 you would pass sizeof(RECORD) * 2 == 32  to cacheDmaMalloc. The function hardly could allocate 32 MB with that information...

Before I could make a reasonable wrapper class I would need your requirements and prepositions. In terms of interfaces and not in terms of wrongly designed implementation code. Beside of the call to DMATransfer2 - without casts - , there is not a single statement in the code above, I would like to use in a wrapper class.

Regards, Alex


| Why should they have such an odd limitation? Isn't that a simple constant that you could overwrite?
It's the requirement of the DMA engine.  This is at a hardware level and is beyond my control.  The documentation/manufacturers requirements clearly state that max RECORD size is 15 MiB.

To DMA anything beyond 15 MiB.  The user needs to create additional records, up to 10 Maximum.  This means I could only DMA 150 MiB of data.  The terminology used to describe this is 'chain' DMA.  So the user create chain records or however they put that.

| Why not simply allocate two buffers by two calls?
Not sure if I follow you here.

| Why do you think you need to bother with offsets and record index? Why do you think you need 32 megabytes?
OK.  Pick a number.  Lets suppose I want to DMA from a source to a destination address 20 MiBs (could be 30 could be 90 .. you get the point) of data (see below for more).

| The function hardly could allocate 32 MB with that information...
It's not intended to allocate 32 MiB.   The intent here is to allocate memory for/create the 'RECORD' that'll be passed to the driver code.  So now here's the steps.   I'm required to FILL the 'record'.   This 'record' will be passed to the DmaTransfer2 (vendor API) function and magic happens.   The DmaTransfer2 function also requires the starting address of the source and destiation addresses.

The steps:
 I determine if the size of my data is greater than 15 MiB.  20 Mib > 15 MiB.   Yes,  I need two - so called - records.  To do this I allocate memory for two records  RECORD *record =  cacheDmaMalloc(2 * sizeof(RECORD));    

Now fill the records:

record[0].size         =  15 MiB
record[0].SrcAdd     =  some_source_address
record[0].DstAdd     =  some_dest_address
record[0].NextRecPtr  = &record[1]

record[1].size         =  5 MiB
record[1].SrcAdd     =  some_source_address  + 15 MiB
record[1].DstAdd     =  some_dest_address  + 15 MiB
record[1].NextRecPtr  = 0. // no more


Now give the 'stuff' to the DmaTransfer2 function.    
DmaTransfer2 (some_source_address, // soruce addr
                       some_dest_address,    // destination addr
                       record[0]);                  // start of the record

You want to DMA 10 MiB of data .. well I create 1 record

  RECORD *record = cacheDmaMalloc(1 * sizeof(RECORD));

record[0].size         =  10 MiB
record[0].SrcAdd     =  some_source_address
record[0].DstAdd     =  some_dest_address
record[0].NextRecPtr  = 0 // No MORE


Bye the way:  cachDmaMalloc is really - I think - 'malloc' under the hood
>>>> This is at a hardware level and is beyond my control.

If it is so, you have a header where the maximum is defined as a const. Or you call a function passing a 20 MB size and getiing an error stating "too much DMA memory required"  or you have a reference manual where the limitation is explicitly defined

>>>> Why not simply allocate two buffers by two calls?
>>>> Not sure if I follow you here.

if *one* call to DMA gives a maximum of 15 MB for one record, why not have a second call that requires another 15 MB? You said, that you have a limitation to 10 records. Ok. But what is the limitation if always allocating *one* record, or would you always get the same memory(addresses) then?

>>>> The intent here is to allocate memory for/create the 'RECORD' that'll be passed to the driver code

True. Do you know why using cacheDmaMalloc for that purpose. The return is a simple pointer and the size isn't relevant. Is there a reason why not using an allocation by operator 'new' or 'malloc' or using a static array

   RECORD recs[2];

>>>> record[0].SrcAdd     =  some_source_address
>>>> record[0].DstAdd     =  some_dest_address

Where do you get 'some_source_address' and 'some_dest_address' ?



Regards, Alex

BTW, have to leave now. Til tomorrow.


|If it is so, you have a header where the maximum is defined as a const.
|Or you call a function passing a 20 MB size and getiing an error stating
| "too much DMA memory required"
|  or you have a reference manual where the limitation is explicitly defined

The manual explicitly states that the max transfer size of is 15 MiB.  That does mean you can't transfer more than 15 MiB from a source to a destination address.  It just means that when you set up your record the Max value you give to the size parameter is 15 MiB.
If you don't it dont work.   So now.

record[0].size         =  19 MiB    // GREATER than the 19 MiB.
record[0].SrcAdd     =  some_source_address
record[0].DstAdd     =  some_dest_address
record[0].NextRecPtr  = 0 // No MORE

  DMA_STATUS stat =  DmaTransfer2 (some_source_address, // soruce addr
                       some_dest_address,    // destination addr
                       record[0]);                  // start of the record

  if ( stat == DMA_SUCCESS )
     //
  else  {
  }
You end up in the 'else' and you could dump the stat parameter.  DMA_STATUS is an enumeration hence stat outputs 'record size larger then allowed' or something to that effect.

| if *one* call to DMA gives a maximum of 15 MB for one record, why not have a
|  second call that requires another 15 MB? You said, that you have a limitation
| to 10 records. Ok. But what is the limitation if always allocating *one* record,
|  or would you always get the same memory(addresses) then?

All I do is have a function that the user will call.  The user will give me the source and destination address and the size of the transfer.  I will create the record and perform the DMA.   So my interface looks like:

   bool dma_xfer( unsigned int * source, unsigned int* dest,  unsigned int size)

So now why have two calls?  The user just calls the dma_xfer function and I create the records based on the size.

Now as for the limitation on 10 records.  I have to look more into this on why 10 - per the documentation.  This implies I can only dma (15 MiB max * 10 records) 150 MiB.  That said if the user passed in 150 MiB as 'size' then swell.  If not I'll tell the user , hey I cant do it (dma_xfer returns false momentarily).
Of course to think someone would want to DMA 150 MiB of data :) .. whew!!


| Do you know why using cacheDmaMalloc for that purpose
cacheDmaMalloc creates some type of 'cache safe buffer'.   That said the CPU will not mess with that area of memory allocated with cacheDmaMalloc.  I've used 'new' and dma'd with success.  Data wasn't corrupt so I'll inquire again about the use of cacheDmaMalloc verus new.  

| Where do you get 'some_source_address' and 'some_dest_address' ?
Lets assume I want to perform memory to memory DMA's.   Lets assume the size is 10 MiB.

So I allocate memory large enough:
  size_t len(15 * 1024 * 1024);
  try {
    int *source = new int[len];   // source pointer
  } catch (std::bad_alloc& err) {}

  std::fill (source, source + len, 0xFFFFFFF);   // fill the source address with a pattern

Allocate a destination address of equal size:
  size_t len(15 * 1024 * 1024);
  try {
    int *dest = new int[len];         // destination pointer
  } catch (std::bad_alloc& err) {}

Now, assuming all's well lets do the dma.
   bool success = dma_xfer( source, dest, len);

The function dma_xfer now sets up the record.  Only one record needed since size/len is equal to max record size.  So now:

   record[0].size         = len
  record[0].SrcAdd     =  source
  record[0].DstAdd     =  dest
  record[0].NextRecPtr  = 0 // No MORE
 
Give record to dma function:
  DMA_STAUTUS stat =  DmaTransfer2 (source,             // soruce addr
                                                          dest,                // destination addr
                                                         record[0]);        // start of the record
I only wish they'd have made the parameters in the record struct size_t.  It's ridiculous to be flipping between unsigned int and size_t and getting warnings.  I suspect the solution is to just stick with the vendors unsigned int.

Thanks for your patience.  I know it takes me a couple iterations before some things sink in but bear with me :)

   
SOLUTION
Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial

| It's unclear, why the DMATransfer2 call needs source, destination and chunk size as arguments.
Agreed, this 'C' interface/library that I'm forced to use is a basket case.   I have no idea why I need to provide source and destination address when all that could be obtained from 'record[0]'.  It is what it is and works so.......

| Tell me, if you agree on the analysis.
Your summary is correct.  It didn't dawn on me that all I'm doing is creating a 'linked list'.   I suspect I should have referred to that from the outset.

| You also could have a constructor where the destination buffer was passed by the caller
I'll need this since the DMATransfer2 needs destination buffer.

| What do you say?
Works for me
Here it is (tested as far as I could)

//dma_xfer.h

#include <iostream>
using namespace std;

#ifndef DMA_XFER
#define DMA_XFER

enum        { MAX_RECORDS = 10, MAX_SIZE_RECORD = 1024 * 1024 * 15, ENG_0 = 1 };
enum STATUS { OK, ERROR };

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

// prototypes (?)
void * cacheDmaMalloc( size_t bytes /* number of bytes to allocate */ );
void   freeDmaMalloc( void* );
STATUS DmaTransfer2(int, unsigned, unsigned, size_t, RECORD* );


template <class T>
class dma_xfer
{
    const T*    source_ptr;
    T*          dest_ptr;
    int         size;  
    bool        dest_alloc;
public:
    dma_xfer(const T* src, int siz, T* dst = NULL)
        : source_ptr(src), size(siz), dest_ptr(dst), dest_alloc(dst == NULL)
    {
        if (dest_alloc)
            dest_ptr = (T*)cacheDmaMalloc( siz);
    }

    ~dma_xfer()
    {
        if (dest_alloc)
            freeDmaMalloc( dest_ptr );
    }

    T* getDest() { return dest_ptr; }

    bool isValid()
    {
        return dest_ptr != NULL && 
               size > 0 && size <= MAX_SIZE_RECORD * MAX_RECORDS;

    }


    bool transfer()
    {
        if (!isValid())
            return false;  // need some error logging here

        int fRec = size / MAX_SIZE_RECORD;
        int nRec = fRec;
        int rSiz = size % MAX_SIZE_RECORD;
        int nSiz = size;
        if (rSiz > 0)
        {
            nRec++;
            nSiz = MAX_SIZE_RECORD;
        }
        else
            rSiz = MAX_SIZE_RECORD;

        int dst  = ((int)dest_ptr)   + fRec * nSiz;
        int src  = ((int)source_ptr) + fRec * nSiz;
        int nxt  = 0;

        RECORD* recArr = new RECORD[nRec];
        for (int i = nRec-1; i >= 0; --i)
        {
            RECORD rec = { rSiz, dst, src, nxt };
            recArr[i] = rec;
            dst  -= nSiz;
            src  -= nSiz;
            rSiz  = nSiz;
            nxt   = (int)&recArr[i];
        }

        STATUS status =  DmaTransfer2 (ENG_0, (unsigned int)source_ptr, (unsigned int)dest_ptr, nSiz, recArr);
        return status == OK;
    }

};    

#endif // DMA_XFER

// main.cpp

#include <iostream>
using namespace std;

#include "dma_xfer.h"

void * cacheDmaMalloc( size_t bytes /* number of bytes to allocate */ ) { return new char[bytes]; }
void   freeDmaMalloc( void* p ) { delete [] (char*)p; }
STATUS DmaTransfer2(int, unsigned, unsigned, size_t, RECORD* ) { return OK; }

int main()
{
    char* src = new char[32 * 1024  * 1024];

    dma_xfer<char> xfer(src, 32 * 1024  * 1024, NULL);
    bool ret = xfer.transfer();
    char* dst = xfer.getDest();
    return ret? 0 : 1;
}

Regards, Alex

Alex, thanks for the response.  Once again I need to think in terms of objects :).  Make sense though to make dma_xfer it's own class.  I'll review and test in the next couple days.   In the interim I have a question - or two for you.  Consider:

class test2 {
  bool success;
public:
  test2() : success(false) {
    success = true;
  }
 void use_object() {}
  operator bool() {  
    return success;
  }
};

int main()
{
  bool x = test2();                                           /// 111
  if ( x == true )                                             ///  111
    std::cout << " success " << std::endl;

  //test2 t;                                                    // 222
  //bool x1 = t;                                        /// 222
 // t.use_object();
}

The conversion operator allowed me to turn a test2 (object?) into a bool.  So far so good?    Trouble is:  The current approach - marked 111 -  does not provided access to the test2() object.   IOW:   With the approach marked -- 222 - I could use t, such that I could call the 'use_object' member function.  My desire is to do a one shot initilization - if you will.   Meaning I need to do something akin to item 222 with approach 111.    My issue with 222 is the user could easily skip the second line i.e   //bool x1 = t;

No I know this wont work but here's - the short story - on  what I'm after:
   bool x = test2 t;  
   // now use t.
 
---------------------------  2 ---------------------------
Consider the source snippet:
# include<vector>

struct test {
  vector<int*> vpi;
  void execute() {
    int *p = cacheDmaMalloc[0x1000];
    vpi.push_back(p);
  }
  ~test()
  {
      for (size_t idx(0); idx < vpi.size(); ++idx)
      {
        cacheDmaFree (vpi[idx]);
      }
      vpi.clear();   /// clear it just in case
  }
};


int main()
{
 test t;
 for (int idx(0); idx < 5; ++idx)
   t.execute();
}


Recall that - and I really need to check this - cacheDmaMalloc is malloc under the hood.  cacheDmaMalloc's cousin is cacheDmaFree.  So now you allocate memory with cachDmaMalloc.  You free said memory with cacheDmaFree.  With regards to the source snippet.  Each call to the function execute results in a dynamic allocation on the free store.  That said, in order to free the allocated memory I store the pointers in a vector.   I'm told that when faced with a scenario like the above, the ideal thing to do is store in a vector of vectors.  So now:


std::vector< std::vector<int> > vpi;
   void execute() {
     vpi.push_back( vector<int>() );
   }

I dont get it.   Why a vector of vectors?  With this approach too I'm now storing 'raw' address which really is the same as pointers but .. I'm puzzled.   Could you elaborate on this:   Replace cacheDmaMalloc with new and use an iterator to delete the contents of the vector in the destructor if you want to use source.
>>>> bool x = test2();

That statement - though syntactically correct - is nonsense. The right side creates a temporay object of class test2(), that was tested by operator bool and the result was assigned to x. A senseful use of that would be

   test2 t2;
   t2.doSomething(...);
   if (t2)  // check if t2 is valid after that
   {
       ...
   }

A good sample for that are iostream derived classes like ifstream that could be checked after open, read like

    ifstream ifs("test.dat");
    if (ifs)
       ...


>>>> I'm told that when faced with a scenario like the above, ... store in a vector of vectors.

A vector of vectors is the means to have a 2D-array by using standard containers. Then, all allocation stuff was done by std::vector (actually you could supply an allocator that would use cacheDmaAlloc but that is really hard stuff, rarely used by application software). An alternative to using vector of vectors is that what you've done in the code snippet: you used a vector of int pointers where any pointer is an - fixed sized - array of ints allocated by cacheDmaAlloc and have to been freed by cacheDmaFree. The decision what alternative is better, depends on the advantages of cacheDmaMemory before 'new'. If there isn't any significant advantage I would prefer vector of vectors as you would avoid a change of paradigmas (malloc against new) and don't have to care about freeing memory.

Regards, Alex

| A good sample for that are iostream derived classes like ifstream that
| could be checked after open, read like
Got it.


| If there isn't any significant advantage I would prefer vector of vectors as you
| would avoid a change of paradigmas (malloc against new) and don't have
| to care about freeing memory.
Use a vector of vector on the test class above and show me source on your usage.
Thanks



Alex,  An additional post to add to my previous above.  

I've reviewed and re-reviewed our persistent design.  I've come along way and learned alot.  Putting a sound design together is the most difficult part for me but all this has been a tremendous blessing.   Anyways, we're familiar with:

class persistent {
//later:
public:
  static bool registerClass(
        const std::string& className,
          CreateInstanceFunc createFunc
        );

};

Now which version of these is the one desired here.  
///  1
    persistent::registerClass("ric_data",    // incoming data
          &ric_data::ric_data_class
        );

/// 2
    persistent::registerClass("ric_data",    // incoming data
          ric_data::ric_data_class
        );

where
struct ric_data {
{
   static persistent* ric_data_class() { return new ric_data; }

};

Something tells me 1.  i.e the address of the function.

------------------- 2 -------------------
On the receive side of things, i'm still unsure why it's necessary to delete the pointers.  Two scenarios:

1.

  ric_data* ptr_msg =
     (ric_data*) (persistent::createInstance(stream));
  // call loadFromStream
  // delete ptr_msg

createInstance returns the address of the object stored in the map.    Hence it would appear to me that the line 'delete ptr_msg' is not necessary here.   In a destructor yes.    Similarily

2.
  persistent *p = persistent::createInstance(stream);
  if ( p )
  {
    p->execute();
    delete  p;
  }
The usual caveat.  Delete in a destructor is understood.  Now again all we're obtaining is the address of the object stored in the map.   It's safe to state that with each call to createInstance (see below), the return statement below does not result in a call to ric_data_class (above) , hence memory is not allocated on a continuum.  So far so good?

persistent* persistent::createInstance(const data_stream& ds)
{
  // later
  return (it->second)();
}

Bottom line again.  Why not delete in a destructor.  Of course, what's even more confusing is the fact that the call to delete p can't possibly delete the memory that was 'registered' via the call:
    persistent::registerClass("ric_data",   ric_data::ric_data_class );  

That said what is being deleted?  
IOW:  
1.  (it-second)(); returns the memory/address of the - ric_data - object to p.
2.  p deletes the memory.  Why should it (p - delete the memory) ?

Conversly, if p deletes the memory.  The next call to createInstance should return an invalid address?  I'm confused?
>>>>  I'm confused?

Yes, you are talking of objects where we have functions.

   persistent::registerClass("ric_data",    // incoming data
         ric_data::ric_data_class
       );

Here, not an object or a pointer to an object was registered but a pointer to the create function. ric_data::ric_data_class is a function pointer of type CreateInstanceFunc.


>>>> Something tells me 1.  i.e the address of the function.

I think both will work, but I prefer (2).

>>>> createInstance returns the address of the object stored in the map.

That is wrong. It calls the create function stored in the map, thus creating an object by 'new'. That's why you need to delete it.

>>>> that the call to delete p can't possibly delete the memory that was 'registered'

Wrong again, there isn't any memory registered but only a function pointer.

>>>>  (it-second)();  

calls create function and returns a pointer to a new object.

Regards, Alex


Ahhhhh!!! Ok

Any take on my post here 06/05/2005 09:22AM PDT?   I understand a vector of vectors.

int main()
{
   // vector of 10 vectors of 20 ints (2D)
  vector<vector<int> > vec2D(10, vector<int>(20));

    // vector of 10 vectors of 15 vectors of 20 ints (3D)
   vector<vector<vector<int> > > vec3D
        (10, vector<vector<int> >(15, vector<int>(20)));

    // typedefs for easier use
    typedef vector<int> vi1D;
    typedef vector<vi1D> vi2D;
    typedef vector<vi2D> vi3D;


    vi2D vec2(10, vi1D(20)); // '2D' : 10 x 20

    vec2[5][8] = 42;     // OK
//   vec2[10][2] = 99;    // Goes from 0-9, 10 is out of range, undefined behavior. I think
//  vec2[4][20] = 25;    // 20 is out of range, undefined behavior


    vec2.at(5).at(8) = 42;    // OK
//  vec2.at(10).at(2) = 99;   // 10 is out of range, throws exception
//  vec2.at(4).at(20) = 25;   // 20 is out of range, throws exception
}

What I dont understnad is the use of vector of vectors for my post here 06/05/2005 09:22AM PDT
# include<vector>

struct test {
  vector<vector<int> > vpi;
  void execute() {
  vpi.push_back(vector<int>(0x1000));
  };
 
  ~test()
  {
         // don't need to clear
  }
};


int main()
{
 test t;
 for (int idx(0); idx < 5; ++idx)
   t.execute();
}


>>>> vpi.push_back(vector<int>(0x1000));

Isn't much efficient cause push_back makes a copy of the temporary vector, so the vector was allocated twice. Better is to have a fixed sized vector or create by new:

  vector<vector<int> >* alloc2DArray(int x, int y)
  {
        return new vector<vector<int> >(x, vector<int>(y));  
  }

You also could do that:

class test
{
       int m_x, m_y;
       vector<vector<int> >& m_v;
public:
       test(int x, int y)
          : m_x(x), m_y(y), m_v(*alloc2DArray(x, y)) {}
       ~test() { delete &m_v; }
};

Of course alloc2DArray could be a static member of class test.

Regards, Alex
Alex, on closer inspection (trying to add comments to the code now) of the persistent class, I have a question for you:  

It seems weird to me that the persisent::createInstance() returns a pointer to persistent, and the objects created thus have to derive from the factory.  Isn't it safe to state that factories usually return a pointer to the object created, which aren't derived from the factory?

Alex one other thing in addition to my previous post.  Recall getBuffer.
  int getBuffer(char*& buf, int siz)
  {
    int sizNeeded = getBufferSize();  //  (aa)
    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;
  }

and the alternative transferbuffer.  So now:
 struct transferbuffer {
  // later
    char* writeableBuffer(int neededSiz)
     {
          if (siz < neededSiz)
          {
               char* p = new char[neededSiz];
               memset(p, 0, neededSiz);
               if (siz > 0)
                   memcpy(p,  buf,  siz);
               delete [] buf;
               buf = p;
               siz = neededSiz;
          }
          return buf;
     }
  };

memcpys in getBuffer accounted for the classId.  It'd seem to me that memcpys in writeableBuffer need to do the same?   if yes, this implies that transferbuffer needs a data_stream object to determine classId.size().   Actually the better way I suppose would be to have a modified getBuffer function pass in the classId size param.  So now:


   bool datastream::getBuffer(transferbuffer& buf)
   {
       buf.writeableBuffer(0, classId.size()); // second argument added to writeableBuffer
   }

It also appears to me that the getBufferSize member function (aa) needs to be passed in as a third argument to writeableBuffer.  Something is amiss about writeableBuffer when compared to getBuffer.
Actually, I have difficulties to recall all issues regarding transferbuffer...

You see the implementations regarding growing are almost identical though transferbuffer simply was a dynamically growing char buffer, while datastream *contains* a stream that can be reconverted by appropriate reverse streaming operations.

The implementation of a growing char buffer isn't very difficult. Hence, I don't know if we *really* need to use both classes...

>>>> It'd seem to me that memcpys in writeableBuffer need to do the same?

No, writeableBuffer simply provides a pointer to a writeable (non-const) char array of the size requested. A 'transferbuffer' has no knowledge of it's contents but only of it's size.

So, the steps are:

  - calculate the needed size of buffer
  - get a writeable buffer of the size requested
  - copy data to the buffer

Regards, Alex

Alex,

Obviously I could initialize a char array 'the hard way'.  Such that:

   char buffer[4096] = {0 };

buffer[0] = 'r';
buffer[1] = 'e';
buffer[2] = 'c';
buffer[3] = 99

The alternative:

    char buffer[4096] = { 'r', 'e', 'c', 99 };

Both of which seem hard when you have a 'decent' amount (say 55)  of elements with different values.  Based on my readings that's as good as it gets.  Correct?

>>>> char buffer[4096] = {0 };

That initializes all array elements.


>>>> char buffer[4096] = { 'r', 'e', 'c', 99 };

You would need

    char buffer[4096] = { 'r', 'e', 'c', 99, 0 };  

to get a terminating zero character and to init the remainder of the array by 0.

    char buffer[4096] = "recc";

is equivalent though the buffer isn't filled up by zeros (depending on compiler).

>>>> Both of which seem hard when you have a 'decent' amount (say 55)  of
>>>> elements with different values.

If it is strings you should consider a string class (buffer) rather than a plain char buffer. But also on char buffers a

    memcpy(&buf[pos], "Hello World", sizeof("Hello World"));

is easier than setting each single character.

Regards, Alex





 

Alex, I suspect I could throw in a slightly unrelated question here.  Note though that I kept this question open - for our correspondence based on current/past work  - and I'll increase points if need be.

In any event, here's my dilema:
I've got a vendor file slHeader that has the following functions and composite types

// slHeader.h
#ifndef SL_HEADER_H
#define SL_HEADER_H
typedef unsigned int       U32;

typedef enum
{
   SL_SUCCESS,
   SL_ERROR,
} slStatus;

typedef struct _slHost
{
   U32 version;
   U32 (*pmcSite)(U32 pciBus, U32 pciDev);
   void (*init)(void);
} slHost;


typedef struct _slDevice
{
   U32 LinkId;
   U32 BridgeId;
   void (*dBellCallBack)(struct _slDevice *slDev, U32 dBellVal);
   slHost *host;
} slDevice;

typedef struct _slOpen
{
   U32 LinkId;
   U32 *handle;
   void (*dBellCallBack)(struct _slDevice *slDev, U32 dBellVal);
} slOpen;

typedef struct _slConnect
{
   U32 LinkId;      
   U32 Offset;          
   U32 size;              
} slConnect;

typedef struct _slWrMsgInit
{
   void * callBack;
   void * param;
} slWrMsgInit;

typedef struct _slDBell
{
   U32 LinkId;
   U32 dBellVal;
} slDBell;

typedef struct _slWrMsg
{
   U32     LinkId;
   U32     param;       // value to be sent,
                            // only bits 3-31 are used,
                            //the 3 LSBs are reserved and will be zero'd
} slWrMsg;


void slConnectOpen (slDevice  *slDev,
                    slConnect *connStruct,
                    slStatus  *stat);
slDevice *slOpenLink(slOpen *ptrSlOpen,
                         slStatus *stat);

void slDBellWrite  (slDevice  *slDev,
                    slDBell   *ptrSlDBell,
                    slStatus  *stat);

void slWriteMsg    (slDevice  *slDev,
                    slWrMsg   *ptrSlWriteMsg,
                    slStatus  *stat);

#endif
//////////////////////////////////////////////////////////////////////////////////////
In  a class called 'slComm' I've got:

// slComm.h
#ifndef SLCOMM_H
#define SLCOMM_H

# include <vector>
# include <string>
# include "slHeader.h"

using namespace std;   // this is presumably bad in a header file.

class slComm {
  private:
    slStatus        status;
    // static   SEM_ID   signallingSema;
  protected:
    slDevice *ptrSlDev;
    slComm(slOpen& slOpenStruct)
       : status(SL_ERROR)
       , ptrSlDev(0)
   {
      ptrSlDev = slOpenLink(&slOpenStruct, &status);
      if (status != SL_SUCCESS)
      {
         return;
      }
     // intialize a semaphore
     // signallingSema = semBCreate (..);
   }
  public:
       static void dBellIsrCallBack (
        slDevice *ptrSlDev,
        unsigned int    dBellVal
      )
   {
     // give a semaphore
    // semGive (signallingSema );
    }

    bool dBellWrite(
       slDBell& slDBellStruct
     )
   {
      // call slDBellWrite function for signaling
   }

    bool writeMsg(
        slWrMsg& wrMsg
      )
   {
      // call slWriteMsg function for signaling
   }

  // SEM_ID  signalingFunction ()      const {
  //       return signallingSema;
  // }

   virtual ~slComm() = 0;
};

class sndr : public slComm {  
  private:
    vector<slConnect> vecSlOpenStruct;
    slStatus        status;

  public:
    sndr( slOpen& slOpenStruct,
            slConnect& slConnectStruct)  
    : slComm(slOpenStruct)
    , vecSlOpenStruct()
    , status(SL_ERROR)
   {
      for (size_t idx(0); idx < vecSlOpenStruct.size(); ++idx)
      {
         slConnectOpen(ptrSlDev, &vecSlOpenStruct[idx], &status);
         if (status != SL_SUCCESS)
            return;
     }
  }
};

class rcvr : public slComm {
   // for simplicity we'll ignore this ..
};
#endif

//////////////////////////////////////////////////////////////////////////////////////

Now here's the basic premise.
Consider the case where we have a sender and a receiver.  Two separate cards connected by RJ45 cables.  

The receiver - by creating an instance of rcvr - will configure a destination (it's all PCI bus type thing) channel.  This destination channel is basically an address where the sender will transmit data.

Assuming a successful connection to the receivers address, the sender - by creating an instance of sndr - have a handle to receivers address/destination channel.

Typical usage:

  // set up the openStruct
  static slOpen myOpenStruct = { 0x10000000,
                               &slComm::dBellIsrCallBack          //  11
                              };
  // set up the connect struct.
  slConnect  connect;
  connect.LinkId       = 0x10000001;  // link id of the receiver
  connect.Offset       = 0;  
  connect.size          = 1024 * 1024;  // The size the sender will use.
                                                    // the receiver might have a larger size but that's fined

So now:
1. The sender will dma data from the senders local memory to the receivers address.
2. Call the dBellWrite member function to inform/signal the receiver there's data available.
3.  The receiver who's 'pending' on a semaphore will wake up and 'grab' the data.  How this works:   The slComm code runs on both the sender and receiver.  The sender and the receiver use the dBellIsrCallBack member function as 'signaling' mechanism.   To do this either sender or receiver must perform item 2 above.   Once item 2 is done, the dBellIsrCallBack member function issues a semaphore  - via the semGive function in the member function dBellIsrCallBack.    So a typical receiver looks like:

    rcvr rec;
    while (semTake ( rec.signalingFunction(), WAIT_FOREVER) )
    {    
        // validate data.
       rec.dBellWrite( .. ) // call doorbell Write function - handshake to sender
       
    }

Where semTake, semGive, SEM_ID and WAIT_FOREVER is specific to VxWorks RTOS.

A typical  sender

1.   sndr sender;    // instance
2.   dma data to receivers address.   Note the destination address - ie. the receivers address -  is the 'handle parameter' in the slOpen struct.
3.  Call the doorbell write member function.  Give signal to receiver.
     sender.dBellWrite( .. ) // call doorbell Write function - tell receiver there's data there
4. Wait for confirmation from receiver
     if semTake ( sender.signalingFunction(), 10 )) {} // on the senders side WE dont wait forever.  We'll wait 10 clock ticks or some amounts of clock ticks..

---------------------------------------------------------------
So thats the scenario in a nut shell.  Hopefully that was understandable.  The source code slComm and slHeader compiles.   What I need is a design that'll allow me to have GREATER control over the callback function.  The use of the callBack function in slComm is too restrictive.  I'd prefer to have the user specify HIS call back function.  IOW: as opposed to specifying the callback as I've done (11 above) - I'd prefer to have the user specify his own in his/her own class.   How could I achieve this?


   





>>>>  // set up the openStruct
>>>>  static slOpen myOpenStruct = { 0x10000000,
>>>>                               &slComm::dBellIsrCallBack          //  11
>>>>                              };

Instead of slComm::dBellIsrCallBack you can pass any other static member function (no matter what class) or a global function. The problem is: whatever substitute you take you need to overtake the job of the original callback function,  i. e.  signalling the semaphore. So, it might be a bad idea to 'have the user specify HIS call back function' cause he/she need most likely don't know what to do.

Maybe better is you provide a new callback function that is calling the original slComm::dBellIsrCallBack *AND* provides an interface before and after calling that the user may override. I would suggest to use a class that my be deived by the user and has some virtual functions to overload. However, to achieve that you'd need to manipulate the first member of slOpen. Instead of passing the sender's ID you'd need to pass a baseclass pointer to the new class. The sender's ID can be a member of the base class. So, you would be able to invoke the original callback function prior or post to calling customized interface functions.

Regards, Alex

Alex, you gave me some ideas.  This post will be wrought with source code - that compiles, nonehtless here goes.

// file called slComm.

#ifndef SLCOMM_H
#define SLCOMM_H

# include<vector>
# include<string>
# include<memory>
# include "slHeader.h"

using namespace std;

class CallbackBase
{
public:
  virtual void operator()() const { };
  virtual ~CallbackBase() = 0;
};
CallbackBase::~CallbackBase() { }

template<typename T>
class Callback : public CallbackBase
{
public:
  typedef void (T::*F)();
  Callback( T& t, F f ) : t_(&t), f_(f) { }
  void operator()() const { (t_->*f_)(); }

private:
  T* t_;
  F  f_;
};

template<typename T>
Callback<T> make_callback( T& t, void (T::*f) () ) {
  return Callback<T>( t, f );
}

template<class T>
std::auto_ptr<CallbackBase> new_callback( T& t, void(T::*f)() ) {
   return std::auto_ptr<CallbackBase>( new Callback<T>(t,f) );
}

 // create a wrapper for the open stuct.  We wont give the user the option to specify his/her callback here.
struct slOpenWrapper {  
  U32 LinkId;
  U32 *handle;  
};

class slComm {
  private:
       static void dBellIsrCallBack (
        slDev *ptrSlDev,
        unsigned int    dBellVal
      )
   {
     // call the user function passed in to the
     // constructor of the sndr & slComm.
     //CallbackBase& cb = *static_cast<CallbackBase*>(&my_cb.get());
     //cb();
   }

   std::auto_ptr<CallbackBase> my_cb;
  protected:
    slDev *ptrSlDev;
    template <typename T>
    slComm(slOpenWrapper& slOpenStructWrapper, T& t, void(T::*f)())
        : my_cb( new_callback(t,f) )
        , ptrSlDev(0)
    {
      // call vendor function with dBellIsrCallBack as callback function
    }
  public:
    virtual ~slComm() = 0;
};


class sndr : public slComm {
  private:
    vector<slConnect> vecSlOpenStruct;
    slStatus        status;
  public:
    template<typename T>
    sndr(  slOpenWrapper& slOpenStructWrapper,
              slConnect& slConnectStruct,
              T& t,
              void(T::*f)()  )
        : slComm(slOpenStructWrapper, t, f)
        , vecSlOpenStruct()
        , status(SL_ERROR)
    {
      for (size_t idx(0); idx < vecSlOpenStruct.size(); ++idx)
      {
        slConnectOpen(ptrSlDev, &vecSlOpenStruct[idx], &status);
        if (status != SL_SUCCESS)
          return;
      }
   }
};

class rcvr : public slComm {
  // similarily the rcvr will have a templated constructor.
};

#endif  // end slComm.h

-------------------------------------------------------------------------------------------------------

// slHeader.h
#ifndef SL_HEADER_H
#define SL_HEADER_H
typedef unsigned int       U32;

typedef enum
{
   SL_SUCCESS,
   SL_ERROR,
} slStatus;

typedef struct _slHost
{
   U32 version;
   U32 (*pmcSite)(U32 pciBus, U32 pciDev);
   void (*init)(void);
} slHost;


typedef struct _slDevice
{
   U32 LinkId;
   U32 BridgeId;
   void (*dBellCallBack)(struct _slDevice *slDev, U32 dBellVal);
   slHost *host;
} slDev;

typedef struct _slOpen
{
   U32 LinkId;
   U32 *handle;
   void (*dBellCallBack)(struct _slDevice *slDev, U32 dBellVal);
} slOpen;

typedef struct _slConnect
{
   U32 LinkId;
   U32 ChannId;
   U32 Offset;
   U32 size;
} slConnect;

typedef struct _slWrMsgInit
{
   void * callBack;
   void * param;
} slWrMsgInit;

typedef struct _slDBell
{
   U32 LinkId;
   U32 dBellVal;
} slDBell;

typedef struct _slWrMsg
{
   U32     LinkId;
   U32     param;  
} slWrMsg;

void slConnectOpen (slDev  *slDev,
                    slConnect *connStruct,
                    slStatus  *stat);
slDev *slOpenLink(slOpen *ptrSlOpen,
                         slStatus *stat);

#endif   // end slHeader.h

-------------------------------------------------------------------------------------------------------
// slTest.h
#ifndef TEST_H
#define TEST_H

# include "slHeader.h"
# include "slComm.h"

class test;
static slOpenWrapper theOpenStruct = { 0x10000000,
                                       0
                                     };
class test {
  sndr *send;
   static slOpenWrapper theOpenStruct;
  slConnect            theConnectStruct;
public:
   test()
      : theConnectStruct(slConnect())
   {
      theConnectStruct.ChannId = 0x10000000;
      theConnectStruct.size    = 1024 * 1024;

#if 0
      send = new(std::nothrow)sndr<test>(theOpenStruct,
                                                      theConnectStruct,                                                                          this,                                                                               &test::myCallBack );
      if (!send)
         return;
#endif
    }
   ~test() {}
    void myCallBack() {}
};

#endif   // end slTest.h

-------------------------------------------------------------------------------------------------------
Here's where I'm going with this:
1.  The user will pass in the desired function.  For test purposes said function is 'myCallBack' above.
2.  Within the constructor of slComm, the callback structure passed to the vendors API (not shown) will be the dBellIsrCallBack member function with - yes - slComm.
3.  The dBellIsrCallBack member function will - in turn call the users member function that was passed in to the constructor of 'sndr' and subsequently ' slComm'.

Now comes my dilema.  
a.  How can I call the users function from within the dBellIsrCallBack member function?  Haven't figured out the approach to achieve that.
b.  Within test how do you instantiate an instance of sndr?  Currenltly the commented out (#if 0 / #endif) portion of the code does not work.  The constructor of sndr and slComm is templated which is what's desired.

This allows me to leave the slComm class 'buttoned' up (basically hard code 'dBellIsrCallBack').  I'll - in turn call the - the user's callback class and the user could do whatever they want to do within their own class.

As always , thanks alot.
a.
No, if you don't have the source code you need to pass your (or your user's) callback function and call the original callback function after being called. The information you have to pass needs to be a pointer to a struct that contains both sender id and private information, e. g.  a pointer to CallbackBase.

b.
I don't think that a template constructor in a non-template class is valid (never heard of that ...). Maybe you need to turn sndr to a template class.

Regards, Alex
| No, if you don't have the source code you need to pass your (or your user's) callback function
| and call the original callback function after being called

It can't work that way.  We prefer for the 'original' callback (which will be hidden from the user) CALL our function, hence my request here.  The other way around from what you proposed.  The question again, is how could i do it in the source provided.    In the mean time I'll try to get additional help on this.

| I don't think that a template constructor in a non-template class is valid (never heard of that ...).
| Maybe you need to turn sndr to a template class.
Ok, I'll see what we need to do different here.
Alex, I could use your guidance on a resolution to an approach here.  

I have a header file called common_header.h.  So  now:

--------------------------------------------------------
--------------------------------------------------------  common_header.h --------------------------------------------------------

#ifndef COMMON_HEADER_H
#define COMMON_HEADER_H

# include <iostream>
# include <string>
# include <map>
# include <bitset>
# include <string>
# include <algorithm>
# include <memory>
#endif

-------------------------------------------------------- --------------------------------------------------------
In a file called 'test.h', I have my template declarations and definitions
--------------------------------------------------------  test.h  --------------------------------------------------------

#ifndef TEST_H
#define TEST_H

# include "common_header.h"
class callback_base
{ };
inline callback_base::~callback_base() {}

template <typename T>
class callback : public callback_base {};

class Base {
  static std::auto_ptr<callback_base> my_callback; };  // static declaration  
std::auto_ptr<callback_base> Base::my_callback;       // static initilization

#endif

-------------------------------------------------------- --------------------------------------------------------
In another file test_.h (NOTE THE UNDERSCORE)
--------------------------------------------------------------- test_.h ---------------------------------------------------------------
#ifndef TEST2_H
#define TEST2_H

# include "common_header.h"
# include "test_.h"
# include "test.h"

class Test {};
#endif

--------------------------------------------------------
// In test_.cpp
-------------------------------------------------------- test_.cpp
# include "test_.h"
// stuff

--------------------------------------------------------
Finally in test_main.cpp, I have
-------------------------------------------------------- test_main.cpp ----------------------------------------------------------
# include "common_header.h"
# include "test_.h"

int main()
{
  Test t;
  t.someOtherFunct();
}


Compiles OK but linker complains at issue is the 'multiple definitions' of my_callback.  The error:

test_main.obj : error LNK2005: "private: static class std::auto_ptr<class callback_base> Base::my_callback" (?my_callback@Base@@0V?$auto_ptr@Vcallback_base@@@std@@A) already defined in test_.obj
test_main.obj : error LNK2019: unresolved external symbol "public: __thiscall Test::Test(void)" (??0Test@@QAE@XZ) referenced in function _main
.\Debug/test_.exe : fatal error LNK1120: 1 unresolved externals

--------------------------------------------------------
How would I get around this while preserving the common_header.h (though I suspect common_header is not the culprit) file.

Note: I found out the the template definitions/declarations also needs to be a header so I'm confused as to what the soluton is.
>>>> std::auto_ptr<callback_base> Base::my_callback;       // static initilization

You need to move that definition to one of the cpp files, e. g. test.cpp.

Generally, you may have 'declarations' as often as you like - though you should protect headers from being included twice by *one* cpp, but you need to havae 'definitions' only once to please the linker.

Regards, Alex



More favors needed :)

Assume a struct .. such that:

struct test {
  unsigned char  low_byte;
  unsigned char  hi_byte;
 };


Two things:
1.   I need a function that'll concatenate the low and hi bytes.  Essentially create a 16 bit word.
2.  Consider the representation:
 
    Bits
      0  ------- 0
      1  ------  20
      2  ------  40
      3  ------  80
      4  ------ 160
      5  ------ 320
      6  ------ 640
      7  ------ 1280
      8  ------ 2560
      9  ------ 5120
      10  ------ 10240
      11  ------ 20480
      12  ------ 40960
      13  -----  0
      14  -----  0  
      15  -----  0  

Lets assume from item 1 that the concatenated value is 1320.   I'll need to set bits 2 and 7 of the representation.
 
IF the concatenated value is 1325, we'll 'evaluate' to determine the closest match.  In this case  bits 2 and 7 (for a total of 1320).     I need to think about an approach (algorithm of sorts) that'll make coding easire...



SOLUTION
Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial

Alex, if I scan the web I could find tons of source for ethernet...   I'd like to think that serial code 232/422/485 should be readily available .. What am I missing?   I have serial code that runs well but it's riddled with 'WINDOWS' 'API's'.  That wont work on my power pc platform.   That said, I need to develop serial code.   I was hoping to find something 'generic'  - ie cross platform.
If you have something let me know.   You could email files to me.
>>>> I'd like to think that serial code 232/422/485

???

>>>> I have serial code that runs well but it's riddled with 'WINDOWS' 'API's'.

There are only a few typedefs and platform specific wrapper functions you easily could hide into one header file that would make the rest  of your code portable. Note, yoe need only a few things in socket interface, the multi-threading calls include mutexes (take CRITICAL_SECTION in  Windows and pthreads elsewhere) and minor parts of time management, e. g. sleep function, where you need to make differences between platforms.

>>>> If you have something let me know.   You could email files to me.  

I have something, but not for the MAC as I never worked on that platform. Emailing code is forbidden in EE (though it seems that we are private in that thread), so we should exchange it  in a public way.

Regards, Alex


|| I have something, but not for the MAC as I never worked on that platform
Sorry, not sure where I mentioned MAC but like you I've never worked on those platforms.


|| Emailing code is forbidden in EE (though it seems that we are
|| private in that thread), so we should exchange it  in a public way.

I see.  This is a paid service and I'd like to believe it's only public to members of EE.     Personally, I prefer an attachment.  In some cases this could be a lot of code..... Anyways, I'll take this up with the moderator.
>>>> That wont work on my power pc platform

What OS are you running there?

>>>> ie cross platform

What platforms (OS/Compiler) do you need to support?

Alex, sorry I was gone for a few days...

| What OS are you running there?
vxWorks and LynX

| What platforms (OS/Compiler) do you need to support?
gcc 2.95

Ideally I'd like a solution that works on windows and Lynx etc.  The trouble with my windows solution is it revolves aroudn the microsoft APIs. We've discussed this before.

Alex .. one additional question.   Consider this source code

# include <iostream>
# include <memory>
# include <string>

using namespace std;
class cb_base
{
public:
  virtual void operator()() const {};
  virtual ~cb_base() = 0;
};

inline cb_base::~cb_base() {}

template <typename T>
class cb_derived : public cb_base
{
public:
  typedef void (T::*F)();
  cb_derived( T& t, F f) : t_(&t), f_(f) {}
  void operator()() const
  {
    (t_->*f_)();
  }

private:
  T* t_;
  F  f_;
};

template <typename T>
cb_derived<T> make_callback (T& t, void (T::*f)() )
{
  return cb_derived<T>(t, f);
}

template <class T>
std::auto_ptr<cb_base> new_callback(T& t, void (T::*f)())
{
  return std::auto_ptr<cb_base>(new cb_derived<T>(t, f));
}


class Base {
  static std::auto_ptr<cb_base> my_callback;
public:
  template<typename T>
  Base(T &t, void (T::*f)() ){
    my_callback = new_callback(t, f);
  }

  static void callback_mem_func() {
    cb_base& cb = *my_callback;
    cb();
  }
};

std::auto_ptr<cb_base>Base::my_callback;
class Derived : public Base {
public:
  template<typename T>
  Derived(T& t, void (T::*f)() )
  : Base(t, f) {
      std::cout << " Derived::Derived " << std::endl;
  };
};

class Use {
  Base *b;
  void some_func(){
    std::cout << " Use::some_func " << std::endl;      
  }
public:
  Use () {
    b = new Derived (*this, &Use::some_func);
  }
  void test() {
    b->callback_mem_func();
  }
  ~Use() {}
};

int main()
{
  Use u;
  u.test();
}

//// Works.. Except now i tried to modify the souce to accomodate the silliness of a vendors library.  

#ifdef      __cplusplus
   typedef void             (*VOIDFUNCPTR) (...);        /* ptr to function returning void */
#else
  typedef void             (*VOIDFUNCPTR) ();                         /* ptr to function returning void */
#endif      /* __cplusplus */

Barring all the uncessary details, the function some_func is the function that the vendors API will call.   What this means is I need to modify the source code to accomodate a function that takes ellipsis.  

So I did:

# include <iostream>
# include <memory>
# include <string>

using namespace std;
class cb_base
{
public:
  virtual void operator()(...) const {};
  virtual ~cb_base() = 0;
};

inline cb_base::~cb_base() {
  std::cout << " cb_base::~cb_base " << std::endl;      
}

template <typename T>
class cb_derived : public cb_base
{
public:
  typedef void (T::*F)(...);
  cb_derived( T& t, F f) : t_(&t), f_(f) {}
  void operator()(...) const
  {
    (t_->*f_)();    // <<< 100
  }
private:
  T* t_;
  F  f_;
};

template <typename T>
cb_derived<T> make_callback (T& t, void (T::*f)() )
{
  return cb_derived<T>(t, f);
}

template <class T>
std::auto_ptr<cb_base> new_callback(T& t, void (T::*f)(...))
{
  return std::auto_ptr<cb_base>(new cb_derived<T>(t, f));
}

class Base {
  static std::auto_ptr<cb_base> my_callback;
public:
  template<typename T>
  Base(T &t, void (T::*f)(...) ){
    //my_callback = new_callback(t, f);  // OR
    my_callback.reset( new cb_derived<T>(t, f));
  }

  static void callback_mem_func(...) {
    cb_base& cb = *my_callback;
    cb();    // <<< 99
  }
};

std::auto_ptr<cb_base>Base::my_callback;
class Derived : public Base {
public:
  template<typename T>
  Derived(T& t, void (T::*f)(...) )
  : Base(t, f) {
      std::cout << " Derived::Derived " << std::endl;
    };
};

class Use {
  Base *b;
  void some_func(...){
    std::cout << " Use::Some_Func " << std::endl;      
  }
public:
  Use () {
    b = new Derived (*this, &Use::some_func);
  }
  void test() {
    b->callback_mem_func();
  }
  ~Use() {
      std::cout << " Use::~Use " << std::endl;      
  }
};

int main()
{
  Use u;
  u.test();
}
...
Works under Visual Studio.  Granted the lines marked 100 and 99 do not specify ellipsis (not sure how I would achieve that).  The question... The code is perfectly legal.  Correct?


Alex, now you have three posts (see my previous two) to respond to.   One more think, while I'm thinking about it.   Within the base class I have 4 callback member functions.   So now:

  static void callback_mem_func1(...) {
    cb_base& cb = *my_callback;
    cb();    
  }

  static void callback_mem_func2(...) {
    cb_base& cb = *my_callback;
    cb();  
  }
 // etc

Also within base I maitain a map.   But first background info.  Within the vendor files,  I have:  

enum CAUSE { CHAN0_COMPLETE, CHAN0_MISS, CHAN0_HIT, CHAN0_ISM };
bool dmaIntConnect( CAUSE c, VOIDFUNCPTR p,  int idx, int jdx);

Typical 'C' usage:
   dmaIntConnect ( CHAN0_COMPLETE, &someFunc1, 0, 0);
   dmaIntConnect ( CHAN0_MISS,         &someFunc2, 0, 0);
   dmaIntConnect ( CHAN0_HIT,            &someFunc3, 0, 0);
   dmaIntConnect ( CHAN0_ISM ,          &someFunc4, 0, 0);

For starters assume someFunc1, someFunc2, someFunc3 and someFunc4 were defined.   The basic premise is that each CAUSE corresponds to a particular function.  

In  a ++ environment, I created a map.  So now base class constructor has within it:

     map<CAUSE, VOIDFUNCPTR> myMap;
     myMap[CHAN0_COMPLETE]   =  &Base::callback_mem_func1;
     myMap[CHAN0_MISS]           =  &Base::callback_mem_func2;
     myMap[CHAN0_HIT]              =  &Base::callback_mem_func3;
     myMap[CHAN0_ISM ]            =  &Base::callback_mem_func4;


I have a function that'll accept a cause and a callback function.  So now:
  template<typename T>
  void connect_isr ( T& t,  void (T::*f)() , CAUSE c);

The function conne_isr when called will search within the map for the appropriate cause.  The cause and member function within the map will then be passed to dmaIntConnect.  So lets assume that the user passed in their desired callback member function and the this pointer to their class.  Lets further assume that the user passed in CHAN0_COMPLETE to connect_isr.  Lastly lets assume the user's callback function is named 'someImportantFunc'.    

The premise:   callback_mem_func1 (which is 'registered' with the vendors API) will in turn call the user's callback function someImportantFunc.  So basically connect ISR is akin to:

  template<typename T>
  void connect_isr ( T& t,  void (T::*f)() , CAUSE c);
  {
     // iterator for map ..  
     //  find cause
     //  if successful call dmaIntConnect( map->first, map->second, 0, 0 )
           my_callback.reset( new cb_derived<T>(t, f));                  // setup user's callback ..
  }


Trouble is:   I need - perhaps - a vector of my_callbacks.   i.e This  - ' static std::auto_ptr<cb_base> my_callback; '  - by itself wont work.   It's evident that if I were to call  connect_isr on 4 occassions - a single 'my_callback' wont suffice.  

Ideas?   You could use compiled source above for illustration.  
   


Come to think of it.. I'm not so sure a vector of auto_ptrs is a good idea..   Containers and auto_ptrs dont go hand in hand..   Anyways, peruse my posts and let me know .. Thanks
>>>> Alex, now you have three posts (see my previous two) to respond to.

Seem to have a busy day today. Maybe I'll find time tomorrow to respond...

Regards
>>>> The question... The code is perfectly legal.  Correct?

Don't know. It's hard to read code that hasn't any comment and makes a lot of funny things, some of them I am not a friend of, e. g. elipses or pointers of non-static member functions typedef'd by a template. brrrrrr) . I need to know what's the purpose of that and I am sure there is amore simple solution as well.

>>>>  virtual ~cb_base() = 0;

Never saw a pure virtual destructor before ... but if it compiles ...

>>>> I'm not so sure a vector of auto_ptrs is a good idea..  

Generally a vector of pointers isn't a good idea beside it were baseclass pointers. IMO, if you already got a map of baseclass pointers you don't need virtual callback pointers cause you could get any static function pointer by an appropriate virtual function call. Or, if you need different callback functions because you need to indentify them and associate them to a derived object when called, then the callback doesn't need to be a member of any of the classes above. It simply could be a global template function like that

template <int i> void callback(int arg1, int arg2, ..)
{
     Base* pb = theManager.getAssociatedObject(i);  // the template id was used in a map of the
                                                                    // manager class (that is a singleton).
     pb->evaluate(arg1, arg2); // you remember execute function ?

     theManager.freeCallback(i);   // put callback back to free list of manager
}

You would need an array of callback pointers. The size of the array is maximum of parallel running callbacks:

typedef void (*Callback)(int arg1, int arg2);


void callback<1> (int arg1, int arg2);
void callback<2> (int arg1, int arg2);
...

Callback cbs[MAX_CALLBACKS] = {  callback<1>, callback<2>, ...};

Regards, Alex
 

|| I need to know what's the purpose of that and I am sure there is amore simple
|| solution as well.
Well then perhapsn you could show an alternative.

Hopefully I'll do a good job conveying the requirments here.  For starters, I'm up against vendor API calls.    The vendor defines in a header file an enumeration called CAUSE.

   enum CAUSE { CHAN0_COMPLETE, CHAN0_MISS, CHAN0_HIT, CHAN0_ISM };

There's a member function caleld dmaIntConnect that takes a cause a VOIDFUNCPTR  and a two integer variables.

   bool dmaIntConnect( CAUSE c, VOIDFUNCPTR p,  int idx, int jdx);

A typical C approach is:
 
# include "vendor_file.h"

// define callbacks
void someFunc1() {
}  
void someFunc2() {
}  
void someFunc3() {
}  
void someFunc4() {
}  

void setup () {
   dmaIntConnect ( CHAN0_COMPLETE, &someFunc1, 0, 0);
   dmaIntConnect ( CHAN0_MISS,         &someFunc2, 0, 0);
   dmaIntConnect ( CHAN0_HIT,            &someFunc3, 0, 0);
   dmaIntConnect ( CHAN0_ISM ,          &someFunc4, 0, 0);
}

The basic premise is that each CAUSE corresponds to a particular function.  
--------------------------------------------------------------------------------------------
In  a C++ environment, I thought well, lets create a map.  So now in a class transfer I thought:

class transfer {
      static  std::auto_ptr<cb_base> user_cb;

     map<CAUSE, VOIDFUNCPTR> myMap;
      void callback_mem_func1(...) {  /* call user function passed in via connect_interrupt */ }
      void callback_mem_func2(...) {  /* call user function passed in via connect_interrupt */  }
      void callback_mem_func3(...) { /* call user function passed in via connect_interrupt */  }
      void callback_mem_func4(...) { /* call user function passed in via connect_interrupt */  }

public:
  transfer() {
     myMap[CHAN0_COMPLETE]   =  &transfer::callback_mem_func1;
     myMap[CHAN0_MISS]           =  &transfer::callback_mem_func2;
     myMap[CHAN0_HIT]              =  &transfer::callback_mem_func3;
     myMap[CHAN0_ISM ]            =  &transfer::callback_mem_func4;
   }

  template <typename T>
  bool connect_interrupt(
    CAUSE c,
    int param1,
    int param2,
    T& t,
    void (T::*f)(...) ) )
  {
    map<CAUSE, VOIDFUNCPTR>::iterator it = myMap.begin();
    for (; it != myMap.end(); ++it)
    {
      if ( it->first == c )
      {
        bool success = dmaIntConnect (
                                          (*it).first,
                                          (*it).second,
                                          param1,
                                         param2
                                       );        
     
        if ( success ) {
            user_cb.reset(new cb_derived<T>(t, f));
          }
      break;
     }
   }  // end for
};

How it works.  

The user will pass to the connect_interrupt function, a cause,  _THEIR_ callback function, and two paramters.  
Within the connect_interrupt function I'll iterate through the map, find the appropriate cause, then call the vendors dmaIntConnect function with a casue, a function and user paramters (param1, param2).  If dmaIntConnect returned success, I 'setup the user's callback.

For instance, callback_mem_func1 will call the user's callback function.   This of course assumes thte user passed in CHAN0_COMPLETE, their desired member function and select parameters.

NOTE:  The callback member function must be an ellipse.  
Thats because in the vendors library.  VOIDFUNCPTR is typedef'd as follows

#ifdef     __cplusplus
   typedef void           (*VOIDFUNCPTR) (...);                   /* ptr to function returning void */
#else
  typedef void           (*VOIDFUNCPTR) ();                        /* ptr to function returning void */
#endif     /* __cplusplus */

How would approach this differently?
Alex you around?
>>>> (*VOIDFUNCPTR) (...);

My compiler help (VC6) says:

If at least one parameter occurs in the parameter list, the list can end with a comma followed by three periods (, ...). This construction, called the “ellipsis notation,” indicates a variable number of arguments to the function.

That would mean the definition above isn't valid cause there is no argument defined beside of the ellipsis.

I made a test

 void f(...)
 {
     cout<< "f(...) called" << endl;
 }
 int main()
 {
    int iff;
    f();
    f(&iff, &iff);
    f(&iff, &iff, &iff);
    return 0;
}

and - to my surprise - it compiled. However, in C/C++ I could not find a way to get the values if there isn't any argument preceding the ellipsis. Normally an ellipse is evaluated in the callee by va_start, va_arg and va_end macros, that make some pointer arithmethics on the preceding argument assuming all variable arguments have same size than the type of the preceding argument. Actually, I don't think that the library actually uses the ellipse argument or if it does it doen't matter whether it is evaluated or not  (e. g. none of your callback functions does evaluate the argument list). I guess, you may get the start address of the call stack by using assembler and these guys used the feature to debug their library ...

Look at the enhancement I made to my sample above:

void f(...)
{
    cout << "f(...)" << endl;

}

typedef void           (*VOIDFUNCPTR) (...);

void g(VOIDFUNCPTR func)
{
    cout << "g(VOIDFUNCPTR)" << endl;

    func(123, 456, 789);
}


void h()
{
    cout << "h()" << endl;
}

int main()
{
    int iff;
    f();
    f(&iff, &iff);
    f(&iff, &iff, &iff);

    g(f);
    g((VOIDFUNCPTR)h);
    return 0;
}

I simply made a cast to the non-ellipsed function h and all work perfect. Even the func call in g make no problems.

>>>> How would approach this differently?

I agree with all you said until it came to function connect_interrupt. Here, things get complex and IMO there isn't any need for that:

>>>>  template <typename T>
>>>>  bool connect_interrupt(

Why using a template? AFAIK, all derived classes are derived from the same baseclass. Why not simply pass a baseclass pointer of the current object and call a virtual function. Remember execute function?

>>>>    CAUSE c,  int param1,     int param2,

ok

>>>>  T& t,

Here, you could pass a baseclass reference of the object that should be called after success.

>>>>    void (T::*f)(...) ) )

Sorry, why passing a function with ellipsis argument????? The callback function isn't passed to the library but is used by your own interface only. Why do yo use an ellipsis that couldn't be evaluated.

If you follow my suggestion and simply call a virtual baseclass function you don't have to bother with function pointers here.

>>>>  {
>>>>    map<CAUSE, VOIDFUNCPTR>::iterator it = myMap.begin();
>>>>    for (; it != myMap.end(); ++it)
>>>>    {
>>>>      if ( it->first == c )
>>>>
>>>>      {
>>>>        bool success = dmaIntConnect (
>>>>                                          (*it).first,
>>>>                                          (*it).second,
>>>>                                          param1,
>>>>                                        param2
>>>>                                       );        

You can replace all that by

       bool success = dmaIntConnect (c, myMap[c], param1, param2 };

if you know, that the map has an entry for each CAUSE item.
     
>>>>        if ( success ) {  
>>>>            user_cb.reset(new cb_derived<T>(t, f));

Ok. Now we are at the critical point. You are using a static smart pointer of a cb_base object and store object t and member function pointer f in cb_derived, a derived template class of cb_base. You need four classes (or more ???)  cb_base, cb_derived, Base, Derived to store *one* pair of object and function into a static smart pointer of a fifth class transfer. Why not make transfer a template class and store a pointer T* and function pointer f directly? What advantage do you have beside of not using a T* but a T& ? If T would be derived from Transfer and has one virtual function - why not call it execute ;-) - you wouldn't need function pointers at all *and* have the advantage that more than one call could run the same time from different threads.

Regards, Alex
Alex, a while back you voiced your displeasure about try/catch handlers.  I was perusing code which was doing something akin to:

void computeLikeCrazy()
{
   if ( condition 1)
   {  
   } else {
      big_flag = false;
  }

   if (big_flag) {
    if (condition2) {} else { big_flag = false;  }
  }

   if (big_flag) {
     if ( condition3) {   } else { big_flag = false;  }    
  }
  // two more .. akin to above
}

 WHen faced with a scenario such as..this.  What do you do/recommend?   Give me the Alex approach :)

Alex you around?
I've got a question.  I've got an implementation here but would like to compare alternatives I suspect.  The question:

For the primitive type char..   Bit's 1 and 2 are the bits of interests.   So now:

  8 7 6 5 4 3 2 1  <- 8 bits

I need to examine bits 1 and 2 to determine if they're zeros or ones.    

  2 ( 1/0 )
  1 ( 1/0)  

How would u achieve this?
You would count bits from 7 to 0 rather than from 8 to 1.

To check a char (== 8bit integer) on bits you would 'AND' it with a constant that has the bits of question set and check whether the result is not 0:

  // check if bit 0 or bit 1 is set
  if ((cvar & 0x03) != 0)    // 0x03 == 0x01 | 0x02
  {
      // check if bit 0 is set
      if ((cvar & 0x01) != 0)
          cout << " bit 0 is set" << endl;
      if ((cvar & 0x02) != 0)
          cout << " bit 1 is set" << endl;
 
  }

Regards, Alex
| You would count bits from 7 to 0 rather than from 8 to 1.
Oops!   Got it..

I might have asked you this before but ... given a vendor struct.

  struct some_struct {
    unsigned int   value1;
    unsigned int   value2;  
    unsigned int   value3;
    // more stuff
  };

 // later
  class X {
    some_struct st;
  public:  
    X() {
      st.value1  =  5;
      st.value2  =  6;
      st.value3  =  7;
    }
  }

Is there a more 'efficient' way to initialize the some_struct besides the approach used in the constructor?
>>>> Is there a more 'efficient' way to initialize the some_struct besides the
>>>> approach used in the constructor?

Don't know exactly if the following is more efficient?

// later
  class X {
    static const some_struct init;
    some_struct st;
  public:  
    X() : st(init) { }              // use copy constructor
  };

// cpp
some_struct X::init = { 5, 6, 7 };

--- or ----

// later
  class X {
    some_struct st;
  public:  
    X()  { some_struct init = { 5, 6, 7 }; st = init; }    // use assignment operator
  };

Regards, Alex



Alex, more questions.   A - once again - vendor function,  returns to me an address.   So now:
  unsigned int* val;
  status  stat = vendor_funct( &val );
 
Typical usage now becomes:
    *val = 0x00000001;    // (1)

The above usage though amounts to though is setting the values in a struct.   So now:

struct my_struct {
  unsigned int  val1          :  1;
  unsigned int  val2          :  2;
  unsigned int  reserved   :  29;
};

val1 from (1) above would be 1.
Here's the dilema.  What I'd like to do is refrain from the approach in 1.  That said, upon receipt of the address I'd like to map/point my_struct to the address returned in val.   Then change things via my_struct.   I'm going to look into an approach called placement new but how else would you achieve this?
>>>> but how else would you achieve this?

I don't understand the problem. An address of size 1 bit is NULL or 0x0000001. But how could the latter be a valid vendor address? Or if it is a handle only - e. g. an index of an array - why do you need bit masking? What has the mapping of a struct like my_struct to do with the contaents of one of its members val1?

Generally, if you get an address from a vendor's function, it most likely there isn't any need to do anything with that value beside of passing it back as an argument of a callback function. If the address is a pointer or an offset to accessable memory, you may try to calculate the pointer and make a cast to the struct you think the result is pointing to. If you know what you are doing, you might be able to manipulate the vendor's functionality by that. But you don't need to allocate a 'new' storage if the address already points to allocated storage. And 'placement new' actually isn't a suitable means to avoid casting.

Regards, Alex

  || I don't understand the problem

Lets try again:

  unsigned int bar0_addr;
  unsigned int addr0;
  unsigned int *control_reg1_addr;
  STATUS stat;

  // later
  bool initialze_board()
  {
    stat = sysPciConfigRead(pci_bus,pci_device, PCI_CFG_BASE_ADDRESS_0,4, &addr0);
    bar0_addr = addr0;
 
    control_reg1_addr = (unsigned int*)bar0_addr;
  }

The function sysPciConfigRead does - in effect a PCI bus translation.  if 'stat' is successful, I obtain a valid 'address' via the member variable addr0.   This address is a 32 bit register in a vendor card where I could write to.    So lets say I want to write the value 2 to the register I'll do:

void some_function (unsigned int wait_time)
{
     // later
     *control_reg1_addr = 0x00000002;   // NOTE how control_reg1_addr was set above
}

The register definition however is - for example as follows:
   
struct my_struct {
  unsigned int  val1          :  1;
  unsigned int  val2          :  2;
  unsigned int  reserved   :  29;
};

My hope then is for something akin to:
  my_struct  m_struct = bar0_addr;

Then use it as follows:
  m_struct.val2 = 2;

The placement new approach to this is as follows:  
  my_struct *m_struct = new (bar0_addr) my_struct;

 Problem is how do you call delete here or wha'ts the alternative.  
It is not all clear but something:


>>>>    control_reg1_addr = (unsigned int*)bar0_addr;

Here you make a cast that means that you would like to interpret the address given in bar0_addr as an pointer to an unsigned int or an array of unsigned int items.

If you know that the storage you got by bar0_addr actually is an object like taht defined by my_struct, simply make a cast to my_struct* instead of unsigned int*

  my_struct* p_control_reg1_addr = (my_struct*) bar0_addr;

That simply tells the compiler that the address effectively points to a my_struct.

   p_control_reg1_addr->val2 = 2;        

You couldn't avoid some kind of a cast cause the vendor passed the address as an integer and not as a pointer. Of course you could copy the address like that:

   memcpy( &p_control_reg1_addr,  &bar0_addr, sizeof(my_struct));

what doesn't look like a cast. But of course a memcpy isn't better but worse.

As I told above you shouldn't use 'placement new' as a substitute for a cast. 'placement new' is a means to have an alternate storage management. You may for example use a shared memory region to allocate data of an array.

Regards, Alex







# include<iostream>
using namespace std;

class base {
protected:
  base() {}
  virtual ~base() = 0 {}
public:
  virtual void override_func() { cout << " override_func " << endl; }
};

class derived : public base
{
public:
  derived() {}
  ~derived() {}
  void do_start() { cout << " do_start " << endl; };
};

int main()
{
  base* b = new derived();     // [1]
  derived *d = new derived(); // [2]
  d->override_func();
  d->do_start();
  delete d;
}

Alex, is there any way to prevent the user from doing [1] above?  The problem with [1] is you have to provide a do_start member function in base but base doesn't need a do_start
I realize I should probably use composition but .. it's a curiosity question.
>>>> The problem with [1] is you have to provide a do_start member function in base

Actually, I don't see a problem. If base didn't provide a do_start function the user will get a compile error when calling it by using a baseclass pointer. So, using a baseclass pointer doesn't give *more* but *less'* functionality what may be wanted or not but doesn't arise a problem, does it?

Regards, Alex

So when dealing with inheritance, it's not mandatory (obviously) that you use a base poitner to a derived.  For some reason I was led to believe that if you're unable to access derived members through a base pointer, something was amiss about your design.   That totally incorrect?

>>>> That totally incorrect?

Yes, and no. Of course virtuality *only* could be experienced by using baseclass pointers or references *but* of course you can use a derived class directly not needing any baseclass pointers.

Regards
Alex,

Consider the case where I've got 3 10 bit samples packed in every 32 bits of data.   So now:
   
  Word    |0              9|           19 |             29 |   | 31|
      0      |  Sample 0  | Sample 1 |  Sample 2  | x | x  |

I need an 'unpacker', such that each sample 10 bit will be stored in a 16 bit word.   I'd also like an unpacker that'll store two samples (Sample 0 and Sample 1) in a 32 bit word.    In this case sample 2 will also be stored in a 32 bit word.  The x's are dont cares.

Alex, in addition to my previous post here's another question for you.
We discussed on 08/18/2005 an implementation that'll take the 'address' returned from a vendors API and map it to a struct.  
So now:

 struct register1 {
   unsigned int value1   :  16;
   unsigned int valu2     :  16;
 };

 register1 *st = (register1*)bar0_addr;

/////////////////
The current approch though is as follows:
  unsigned int *register1 = (unsigned int*)bar0_addr;
 
void process()
{
  unsigned int reg1_val;
  reg1_val = 0xFF00FF00;                 // set reg1_val to FF00FF00
  *register1 = LongSwap(reg1_val);  // update reg1.
}

where LongSwap is as follows:

#define LLSB(x)      ((x) & 0xff)            
#define LNLSB(x) (((x) >> 8) & 0xff)
#define LNMSB(x) (((x) >> 16) & 0xff)
#define LMSB(x)       (((x) >> 24) & 0xff)
#define LongSwap(x) ((LLSB(x) << 24) | \
                 (LNLSB(x) << 16)| \
                 (LNMSB(x) << 8) | \
                 (LMSB(x)))
//
 
///  The question.  For the case :
register1 *st = (register1*)bar0_addr;
// later
void process()
{
  st->val1 = 0xFF00;
  st->val2 = 0xFF00;
}

This poses a potential problem since I need to run the LongSwap macro on the values.  One solution I suspect is.
 
void process()
{
  LongSwap(st->val1) = 0xFF00;
  LongSwap(st->val2) = 0xFF00;
}

Except what do you do when you have individual bits to contend with.  So now:

  struct register2 {
    unsigned int   idx          :   1;  
    unsigned int   jdx          :  1;
    unsigned int   kdx         :  1;
    unsigned int   ldx          :  1;
    unsigned int   reserved  :  28;
  };

register2 *rt = (register2*)bar1_addr;

void process()
{
  LongSwap(rt ->idx) = 1;            /// NOPE
  // more
}

What's the work around?
Alex, you around?
>>>> Alex, you around?

Yes and no ;-)

I am busy today, maybe tomorrow I'll find more time.

I wouldn't use macros like LongSwap but htonl or ntohl (PC) if you need to convert to/from big-endian.

Note, different endian means also different bit order cause a big-endian integer has the most significant bit is stored at the lowest bit address. You don't need to care about bit ordering/swapping cause that is done automatically when reading from an external device, e. g. from a modem or a disk. So, bit masks would be transferred to a different endian system if the sum of bits is 32 or 16 and if you are using the appropriate byte-swapping functions htonl or htons respectively ntohl and ntohs always at the PC system.

Regards, Alex
| I wouldn't use macros like LongSwap but htonl or ntohl (PC) if you need to convert to/from big-endian.
Ok well the LongSwap was provided as part of the vendor header file.  The example source provide used LongSwap so I thougth well I'll conform.   Troubel is I didn't like the C approach.  Works but I'm trying to improve my ++.  Having said that, the current "C" approach - which works is akin to:

 register1 *st = (register1*)bar0_addr;

/////////////////
  unsigned int *register1 = (unsigned int*)bar0_addr;
 
void process()
{
  unsigned int reg1_val;
  reg1_val = 0xFF00FF00;                 // set reg1_val to FF00FF00
  *register1 = LongSwap(reg1_val);  // update reg1.
}

-------------------------------------------------------------------------------------------------------------------
I'm not a fan of having to use 'hard code values' - such as FF00FF00.   That said, since the fields can be easily represented by a struct.   I'll do:

struct register1 {
   unsigned int value1   :  16;
   unsigned int valu2     :  16;
 };

// latre
 register1 *st = (register1*)bar0_addr;   //  [55]
The above gives me direct access to the addresse represented by bar0_addr.  Are you saying that I could easily do:

void process()
{
  LongSwap(st->val1) = 0xFF00;
  LongSwap(st->val2) = 0xFF00;
}
Alex, ignore my previous post.   Accidentially depressed the submit button prematurely.

| I wouldn't use macros like LongSwap but htonl or ntohl (PC) if you need to convert to/from big-endian.
Ok well the LongSwap was provided as part of the vendor header file.  The example source provide used LongSwap so I thougth well I'll conform.   Troubel is I didn't like the C approach.  Works but I'm trying to improve my ++.  Having said that, the current "C" approach - which works is akin to:

 register1 *st = (register1*)bar0_addr;

/////////////////
  unsigned int *register1 = (unsigned int*)bar0_addr;
 
void process()
{
  unsigned int reg1_val;
  reg1_val = 0xFF00FF00;                 // set reg1_val to FF00FF00
  *register1 = LongSwap(reg1_val);  // update reg1.
}

-------------------------------------------------------------------------------------------------------------------
I'm not a fan of having to use 'hard code values' - such as FF00FF00.   That said, since the fields can be easily represented by a struct.   I'll do:

struct register1 {
   unsigned int value1   :  16;
   unsigned int valu2     :  16;
 };

// latre
 register1 *st = (register1*)bar0_addr;   //  [55]
The above gives me direct access to the addresse represented by bar0_addr.  Are you saying that I could easily do:

void process()
{
  st->val1 = 0xFF00;
  st->val2 = 0xFF00;
}

as opposed to:

void process()
{
  LongSwap(st->val1) = 0xFF00;
  LongSwap(st->val2) = 0xFF00;
}

OR are you saying use  htonl or ntohl instead?   Note I'm not using a PC but I coudl drum up reasonable endian conversions instead.

NOTE: Withouth too much details The address that's mapped out in PC space between an FPGA and my processor is the impetus behind the endian conversion.   The processor sees BIG endian.   The FPGA sees littler.
>>>> Are you saying that I could easily do: st->val1 = 0xFF00;  st->val2 = 0xFF00;       (A)
>>>> as opposed to:  LongSwap(st->val1) = 0xFF00; LongSwap(st->val2) = 0xFF00;    (B)
>>>> OR are you saying use  htonl or ntohl instead?                                                    (C)

I would say that A is wrong cause it doesn't change anything *and* works on words rather than on bytes. B is wrong cause whatever good or bad LongSwap does on the left-side 16bit integer (???), you would overwrite it by the following assignment, what assigns the same values as before, so the whole thing  does *nothing*.

(C) would do byte-swapping if made on a little-endian system (htonl wouldn't do anything on your BIG endian system. I think that's the reason for LongSwap macro cause you may easily turn it to a C/C++ function) and you should operate on the original bar0_address integer cause you  don't need casting, then. Generally, I still have problems to understand why an address returned by a vendor's function should need bit masking or 16bit conversion. If any of the systems is a 16bit  system, it still would have 32 bit addresses or an address built by two short integers. Then, I would recommend using htons/ntohs that could deal with 16 bit integers.

Regards, Alex



Alex, lets assume addr = 0x4cFF4000;


So now:
typedef unsigned int uint32_t;

struct Register {
  operator uint32_t() { return *reinterpret_cast<uint32_t *>(this); }
  void set(uint32_t n) {
  *reinterpret_cast<uint32_t *>(this)=LongSwap(n);   // change to htonl or nhtols if we can
  }
};

struct register2 : public Register{
  uint32_t start_vec : 16;
  uint32_t stop_vec : 16;
};

register2 *ptr;
void initialize()
{
  uint32_t addr =  0x4cFF4000;
  ptr = (register2*)addr;
}

void set_fpga_register(
  unsigned short val1,
  unsigned short val2 )
{
  register2 reg;
  reg.start_vec = val1;
  reg.stop_vec = val2;
  ptr->set(uint32_t(reg));  // []1]
}

Do you see any problems with this approach?
>>>> Do you see any problems with this approach?

No, though I wouldn't do it that way.

Why not that way?

typedef unsigned int   uint32_t;
typedef unsigned short uint16_t;

union Unsigned
{
    uint32_t  ui;
    uint16_t  us[2];
    uint32_t  longswap()
    { return ui = ((ui & 0xff) << 24) |
                  ((ui & 0xff00) << 8) |
                  ((ui & 0xff0000) >> 8) |
                  ((ui & 0xff000000) >> 24); }
};

unsigned long addr = 0x4cFF4000;

void t()
{
  unsigned short val1 = 0xabcd;
  unsigned short val2 = 0x4321;

  Unsigned u;
  u.us[0]  = val1;
  u.us[1]  = val2;
  u.longswap();
  memcpy((void*)addr, &u.ui, sizeof(uint32_t));
}

Regards, Alex