Link to home
Start Free TrialLog in
Avatar of naseeam
naseeamFlag for United States of America

asked on

Passing Pointer by reference ?

void somefunc( )
{
   unsigned char * appBuffer;
   /* some code here */

   func1( Id, size, appBuffer, firstPacketMaxsize, packetMaxSize);
   /* some code here */
}

unsigned char func1(unsigned short Id, unsigned short size, unsigned char * & dataBuffer, unsigned short &packet, unsigned short &maxsize)
{
   /* some code here */
   dataBuffer = &_firmwareUpgradeBuffer[0];
   memset(dataBuffer, 0xFF, maxsize);   /* dataBuffer = 0xA0011F18 */
  /* some code here */
}

Open in new window

 


pointer appBuffer is passed by reference.  In func1( ), it is assigned address 0xA0011F18.
When func1( ) returns to  somefunc( ), this address changes to 0xA0010BB0.

Isn't that wrong ? appBuffer address should stay 0xA0011F18 when control is returned to somefunc() ?  Am I correct ?  If yes, what might be the problem ?
ASKER CERTIFIED SOLUTION
Avatar of jkr
jkr
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
BTW, "correctly changes it to mpoint to a different memory area" (sic!) of course only is correct if you meant to do that.
Avatar of naseeam

ASKER

After dataBuffer is changed to point to different memory area, why does it change again?
Is the debugger lieing ?

/* after this statement, dataBuffer is 0xA0011F18 */
dataBuffer = &_firmwareUpgradeBuffer[0];    

then, when you return from function, appBuffer changes to 0xA0010BB0.   appBuffer is assigned any address again.
Avatar of naseeam

ASKER

sorry I meant appBuffer isn't assinged an address again.
That's hard to tell without knowing what

  /* some code here */

Open in new window


actually is. But given that the 'memory distance' is 4968 bytes, this could point to both options. Yet the 'return' alone will not alter the value itself. Ingeneral, passing a reference to a pointer is the same as passing a pointer to a pointer metholigically, so just think of both conceps basically representing the same.
But couldn't it be somehow affected by a loop etc.?
Hmmm, any chance that there is something wrong about the 'memset()' statement - does the value stay correct if you leave that out?
Avatar of naseeam

ASKER

Actually I believe your very first explanation answered the question.  I think I failed to understand it because of my lack of knowledge on passing pointers by reference.  

Attached is the snaphot of dataBuffer and appBuffer.  

I thought dataBuffer should be exactly same as appBuffer but that isn't the case.  Please explain relationship between these two variables.
reference-confusion.PNG
Both these variables should stay the same. 'dataBuffer' is merely an alias used for 'appBuffer' within the scope of 'func1()' since it is a reference to the latter in that very function. That's the whole point of C++ references.
Avatar of naseeam

ASKER

OK.  Thank you!  Now, I'm passing pointer to pointer.  I get unrelated compile error.  Please see attached.

The error is in header file PdpApp.h

class PdpApp: public IPdpApplication
{
  private:
       static const uint32 ParametersSupported = 1600;
       ....
   
  public:
       enum  EventId
      { EventId_None                = 0x00,
        EventId_Timeout             = 0x01,
        EventId_ParameterHasChanged = 0x02,
        EventId_NewEvent            = 0x10,
        EventId_StopEvent           = 0x20,
        EventId_Reset               = 0x40,
        EventId_UploadStatus        = 0x80,
        EventId_UploadStatusEnd     = 0x100,
        EventId_ModBusWrite         = 0x200
     };
    .....
    public:
        virtual bool IsParameterSupported(int32 parameterIndex);
        ....
};

// Singleton instantiation of the class.
extern PdpApp  pdpApp;


#endif
   

Open in new window




Please explain and resolve the compile error.  Passing by pointer to reference didn't  cause this error.  Why passing pointer to pointer causes this error.
compile-error.PNG
Seems that you didn't implement all pure virtual functions of the base class 'IPdpApplication' (at least that's what the error points to). Be sure to overload (== implement) every function that is labeled as 'pure virtual', i.e. 'virtual void Method () = 0;'
When the values of auto variables change randomly it is almost certainly due to stack corruption. If it changes as the function exits it suggests the functions stack frame has been messed up. The memset, as jkr has noted, being the prime suspect.

There is nothing intrinsically wrong with passing a pointer by reference, it's just another data type.
Avatar of phoffric
phoffric

>> Now, I'm passing pointer to pointer.
When doing this conversion, I could imagine a few gotchas. Since you are already in good hands, the only thing as a self-exercise for you that I would suggest is to see if you can convert the following using pointer to pointer, and get the same output.
#include <stdio.h>

void func1(unsigned short Id, unsigned short size, unsigned char * & dataBuffer, 
   unsigned short &packet, unsigned short &maxsize);

void somefunc( )
{
   unsigned char * apB = 0;
   unsigned short Id=0, size=0, firstPacketMaxsize=0, packetMaxSize=0;

   func1( Id, size, apB, firstPacketMaxsize, packetMaxSize);
   printf("apB = %02x %02x %02x %02x", apB[0], apB[1], apB[2], apB[3]);
}

void func1(unsigned short Id, unsigned short size,
            unsigned char * & dataBuffer, unsigned short &packet,
            unsigned short & maxsize)
{
   static unsigned char _firmwareUpgradeBuffer[] = {0xA0, 0x01, 0x1F, 0x18};
   dataBuffer = &_firmwareUpgradeBuffer[0];
}

int main() {
   somefunc();
}
// output is:
// apB = a0 01 1f 18

Open in new window

I realize that this may not be your exact problem (e.g., no memset), but at least it will confirm that you can make the conversion correctly. No need to post anything if you get this to work after the change.
Although you have changed your code to use a pointer to a pointer, you initial code should have been just fine. Usually when this kind of thing happens it is because you wrote past the end of an array or wrote to a dangling pointer or reference. This can cause you to overwrite memory that belongs to a different variable. You should step through your code in the debugger line-by-line and find the exact point at which the pointer changes. Or if the function is very long, add break points to narrow down the location.
I'm pretty sure I've already said this!
Avatar of naseeam

ASKER

I have posted complete base class and derived class.  Please try to resolve the compile error.





 // Filename:  PdpApp.h  class PdpApp: public IPdpApplication
{
  private:
     static const uint32 ParametersSupported = 1600;
    static IParameter **_iParameterList;
    static uint32 _parametersUsed;

    static ModMgrSocket      * _modMgrSocketPtr;
    static const uint32 FirmwareUpgrade1stPacketMaxSize = 240;
    static const uint32 FirmwareUpgradePacketMaxSize = 256;
    static uint8 _firmwareUpgradeBuffer[FirmwareUpgradePacketMaxSize];
    static uint32 _firmwareUpgradeBytesReceived;
    static uint32 _firmwareUpgradeImageSize;

    enum FirmwareUpgradeObject
      { FirmwareUpgradeObject_Monitor = 0x00,
        FirmwareUpgradeObject_Mpac = 0x01,
        FirmwareUpgradeObject_Max
      };

     public:
        enum  EventId
      { EventId_None                = 0x00,
        EventId_Timeout             = 0x01,
        EventId_ParameterHasChanged = 0x02,
        EventId_NewEvent            = 0x10,
        EventId_StopEvent           = 0x20,
        EventId_Reset               = 0x40,
        EventId_UploadStatus        = 0x80,
        EventId_UploadStatusEnd     = 0x100,
        EventId_ModBusWrite         = 0x200
};

     static OS_TID _taskIdPdpApp;
    
       static void PdpAppTask(void);

    
    
    static void UpdateParameterById(uint16 id);
    
    static void UpdateParameterByIndex(uint32 index);

 
   
  public:
    virtual bool IsParameterSupported(int32 parameterIndex);

        virtual int32 GetParameterSize(int32 parameterIndex);

       virtual int32 GetParameterSizeById(int32 parameterId, uint32 objectId);

        virtual ServiceResult GetParameterIndex(uint32 parameterId,
                                            uint32 objectId,
                                            uint32 &parameterIndex
                                           );

       virtual ServiceResult GetParameterValue(uint32 parameterIndex, 
                                            uint32 &objectId,
                                            uint32 &parameterId,
                                            int64  &value,
                                            OpcQuality &opcQuality
                                           );  


       virtual ServiceResult  SetParameterValue(uint32 parameterId, int64 value, uint32 parameterSize, uint32 objectId = 0);

        virtual ServiceResult  SetParameterStatusValue(uint32 parameterIndex, int64 value, uint32 parameterSize, uint32 objectId = 0);


        virtual ServiceResult GetParameterString(uint32 parameterIndex, 
                                             uint32 &objectId,
                                             uint32 &parameterId,
                                             string &value,
                                             OpcQuality &opcQuality
                                            );
                                            
       virtual ServiceResult  SetParameterString (uint32 parameterId, string value, uint32 objectId = 0);


       virtual ServiceResult  SaveParameters();

       virtual ServiceResult  ExecuteDoAction (ObjectType objectId, uint32 actionId);

        virtual UpdateComponentStatus PrepareForComponentUpgrade(uint32 componentId, uint32 imageSize, uint8* &dataBuffer, uint32 &firstPacketMaxSize, uint32 &packetMaxSize);

       virtual UpdateComponentStatus UpdateComponentFirmware(uint32 componentId, uint32 packetSize);

       virtual UpdateComponentStatus GetCurrentStatusOfConponentUpgrade(uint32 componentId);
    
       virtual void AbortConponentUpgrade(uint32 componentId);    

        virtual bool  PrepareForReset ();

       virtual void  ResetDevice ();

       virtual void GetNumberOfLoggedEvents(int64 &eventsLogged, int64 &oldestEventRecorded);

        virtual ServiceResult GetEventInfo(uint64 index,  EventInfo& eventInfo);

       virtual uint32 GetActiveDeviceAlerts(vector <DeviceEvent>& alerts);

}; 

// Singleton instantiation of the class.
extern PdpApp  pdpApp;

// Filename:  IPdpApplication.h

class IPdpApplication
{
  public:

    virtual bool IsParameterSupported(int32 parameterIndex) = 0;

    virtual int32 GetParameterSize(int32 parameterIndex) = 0;

    virtual int32 GetParameterSizeById(int32 parameterId, uint32 objectId) = 0;

    virtual ServiceResult GetParameterIndex(uint32 parameterId,
                                            uint32 objectId,
                                            uint32 &parameterIndex 
                                           ) = 0;
   virtual ServiceResult GetParameterValue(uint32 parameterIndex, 
                                            uint32 &objectId,
                                            uint32 &parameterId,
                                            int64  &value,
                                            OpcQuality &opcQuality
                                           ) = 0;
  virtual ServiceResult GetParameterString(uint32 parameterIndex, 
                                             uint32 &objectId,
                                             uint32 &parameterId,
                                             string &value,
                                             OpcQuality &opcQuality
                                            ) = 0;
  virtual ServiceResult  SetParameterValue(uint32 parameterId, int64 value, uint32 parameterSize, uint32 objectId = 0) = 0;

  virtual ServiceResult  SetParameterStatusValue(uint32 parameterId, int64 value, uint32 parameterSize, uint32 objectId = 0) = 0;

virtual ServiceResult  SetParameterString(uint32 parameterId, string value, uint32 objectId = 0) = 0;

virtual ServiceResult  SaveParameters() = 0;

virtual ServiceResult  ExecuteDoAction(ObjectType objectId, uint32 actionId) = 0;

virtual UpdateComponentStatus PrepareForComponentUpgrade(uint32 componentId, uint32 imageSize, uint8 ** dataBuffer, uint32 &firstPacketMaxSize, uint32 &packetMaxSize ) = 0;

virtual UpdateComponentStatus UpdateComponentFirmware(uint32 componentId, uint32 packetSize) = 0;

virtual UpdateComponentStatus GetCurrentStatusOfConponentUpgrade(uint32 componentId) = 0;

virtual void AbortConponentUpgrade(uint32 componentId) = 0;

virtual bool  PrepareForReset() = 0;

virtual void  ResetDevice() = 0;

virtual void GetNumberOfLoggedEvents(int64 &eventsLogged, int64 &oldestEventRecorded) = 0;

virtual ServiceResult GetEventInfo(uint64 index,  EventInfo& eventInfo) = 0;

virtual uint32 GetActiveDeviceAlerts(vector <DeviceEvent>& alerts) = 0;

Open in new window

Avatar of naseeam

ASKER

Never mind!  It's my problem.  When I was passing pointer to pointer, I forgot to change the argument in the prototype from pointer to reference to pointer to pointer.
Avatar of naseeam

ASKER

Great support!  Thank you for answering so many follow up questions.
>> When I was passing pointer to pointer, I forgot to change the argument in the prototype from pointer to reference to pointer to pointer.
    That was one of the gotchas.
to add to before comments:

a reference to pointer is better coding than a pointer to pointer, same as a reference is better than a pointer, normally.

the reason is that a reference reduces complexity because you have only one variable and not a pointer variable pointing to another variable. you only have to keep in mind that if you pass a reference that it "IS" the same variable the caller has passed as argument even if it has a different name.

with pointer to pointer you have 3 participants: a pointer variable a is pointing to a pointer variable b which points to an object c. moreover, because a pointer may point to the begin of an array, pointer to pointer could also point to the first row of a 2-d array what doesn't make it easier.

generally, returning new pointers from functions by return value or by output argument is not a good design, cause the caller would need to take a responsibility for the pointer which it didn't create. the caller doesn't even know whether the pointer points to non-heap memory (like in your code)  or not. it would be much better to create an (empty) object by the caller and let the function only fill the data to the object.

MyData data;
bool bok = fill(data);

...
bool fill(MyData & data)
{
       ...
}

Open in new window

there is no pointer involved and the design is straight and simple. you even could have a base class and still don't need pointers:

class MyBase
{
protected:
       virtual bool load() = 0; 
       ...
};

class MyData : public MyBase
{
       ... 
protected:
       bool load() { .... }
};

MyData data;
bool ok = fill(data);

...
bool fill(MyBase & data)
{
       return data.load();
}

Open in new window


if you have to store objects of different derived classes in a container, you would need to use pointers.

class MyBase
{
       static std::vector<MyBase*> all;
protected:
       MyBase() { all.push_back(this); }
       virtual ~MyBase() { 
            std::vector<MyBase*>::iterator f = std::find(all.begin(), all.end(), this);
            all.erase(f);
       }

       virtual bool load() = 0; 
       ...
};
class MyData1 : public MyBase
{
       ... 
protected:
       bool load() { .... }
};

class MyData2 : public MyBase
{
       ... 
protected:
       bool load() { .... }
};

MyData2 data2;
bool ok = fill(&data2);

...
void fill(MyBase & data)
{
       return data.load();
}

Open in new window



though the base class pointers were stored in the 'all' container, there is no need to deal with pointers outside of the class.

Sara