macOS Stupid C++ tricks

[Analog Kid]

Hey--

I'm trying to wrap a third party C library in a C++ wrapper, and I'm trying to do it as efficiently as possible. The bulk of the library revolves around a core data structure, which I'll simplify as:

typedef struct Data 
{
 int x; 
 int bigArrayType; 
 char * bigArray;
} Data;

I'd like to wrap that in a class, but I don't want to create a class that contains a Data, I'd like to subclass Data (struct being a special case of class):

class OOData: public Data
{
private:
  inline void initFromDataPtr(Data* d){*(Data*)this=*d;};
public:
  OOData(Data* d){initFromDataPtr(d);};
};

The reason for this is that it allows me to use my class in place of the legacy data structure in existing functions.

The way I'm doing that above requires allocating storage, copying the contents of the Data object I'm wrapping, and then deallocating that object. Is there a way to just use the existing object and rewrap it?

"static_cast<OOData*>d" comes to mind, but that bypasses any constructors of OOData and therefore doesn't allow additional initialization.

Any ideas?

 

[lee1210]

I would be very concerned about getting this to work at all, much less across compilers. I would try to have the structure be a member of your object, and have instance methods that pass that thing down to the existing C functions, and "refresh" the object state as needed after return.

Trying to coerce the compiler into a particular memory layout for an object seems unsafe. I would choose a design that does not rely on such shenanigans, and work on implementing it as cleanly and efficiently as possible.

-Lee

 

[garethlewis2]

friend void operator=(const OOData data, struct Data d);

That should work. This will allow the OOData object to the lref and the Data struct to the be the rref. I on purpose have made the method signature return void. You can change that. If you want the Data as the lref, just add another assignment operator with the parameters the other way round.

 

[Sander]

Why don't you want to make Data a member of your OOData? You could give OOData a constructor which takes a Data and makes an OOData around it, along with a conversion operator which lets you go from OOData to Data. I'm not saying implicit conversions are a good idea (they usually aren't) but it sounds like you want to wrap with minimal code changes in the legacy code.

 

[Analog Kid]

friend void operator=(const OOData data, struct Data d);

That should work. This will allow the OOData object to the lref and the Data struct to the be the rref. I on purpose have made the method signature return void. You can change that. If you want the Data as the lref, just add another assignment operator with the parameters the other way round.

Sorry if I'm slow on this, but I'd still need to copy the members from object to object in the body of the assignment operator, right?

I would be very concerned about getting this to work at all, much less across compilers. I would try to have the structure be a member of your object, and have instance methods that pass that thing down to the existing C functions, and "refresh" the object state as needed after return.

Trying to coerce the compiler into a particular memory layout for an object seems unsafe. I would choose a design that does not rely on such shenanigans, and work on implementing it as cleanly and efficiently as possible.

-Lee

I think you're right as far as trying to repurpose an existing allocation into a more derived object-- most compilers keep the members of more derived objects contiguous with their base members. static_cast would work, assuming OOData doesn't allocate additional fields or require a virtual function table-- but that last bit kills a lot of what I was trying to do.

I don't think code as written is doing anything untoward. OOData is more derived than Data, and therefore is a complete implementation of Data-- copy semantics should work fine.

As implemented, OOData* works just fine as a stand in for functions that previously expected Data*.

I think the key is probably to focus on how bigArray is managed-- the way the routine is written now will copy the pointer value over, and I just have to be sure not to deallocate that block.

 

[garethlewis2]

Yes you would need to copy.

C++ automagically supplies an operator= function that does memberwise assignment, but this only copies references and not memory. Your overloaded version will include the code to copy the memory. You would just new the char array and memcpy the data across.

 

[Analog Kid]

Why don't you want to make Data a member of your OOData? You could give OOData a constructor which takes a Data and makes an OOData around it, along with a conversion operator which lets you go from OOData to Data. I'm not saying implicit conversions are a good idea (they usually aren't) but it sounds like you want to wrap with minimal code changes in the legacy code.

That's right-- I don't want to touch the legacy code because we don't maintain it.

I did think of just keeping Data member and writing an operator Data*() conversion. I think that will work, and I may fall back on it. It just feels less organic-- OOData isa Data, not hasa Data. I can make one mimic the other, but it doesn't feel right.

Truth is though that anything that uses OOData should be written to go through accessors anyway-- so I don't think it's making new code any more complex. It breaks identity checks, but I don't think that's a problem here.

The only other problem I see is that every time I use OOData in place of Data, I'll be adding some overhead. As I've written it now, I only take the hit on construction which is probably less frequent and much less likely inside a loop.

 

[lazydog]

Hi

Not sure I understand your problem but how about this...
You could cheat by defining a copy constructor which did nothing, and free up bigArray in OOData's destructor (or stick a destructor in the struct for Data). You would have to be careful in your code though, if you delete an OOData object then you don't want to call a library function that also freed up the underlying Data struct.

class OOData: public Data
{

public:

   OOData(Data* d)
   {

   }

   ~OOData()
   {
      delete bigArray ;
   }
  
   int get_x() const
   {
       return x ;
   }
} ;

ß e n

 

[Analog Kid]

Hi

Not sure I understand your problem but how about this...
You could cheat by defining a copy constructor which did nothing, and free up bigArray in OOData's destructor (or stick a destructor in the struct for Data). You would have to be careful in your code though, if you delete an OOData object then you don't want to call a library function that also freed up the underlying Data struct.

class OOData: public Data
{

public:

   OOData(Data* d)
   {

   }

   ~OOData()
   {
      delete bigArray ;
   }
  
   int get_x() const
   {
       return x ;
   }
} ;

ß e n

Wait-- does that work? A copy constructor that does nothing? What's happening under the hood there?

Thanks

 

[lazydog]

Wait-- does that work? A copy constructor that does nothing? What's happening under the hood there?

Thanks

Ah sorry ... I was just about to go to the dentists and not thinking straight, but it's been cancelled now

From my understanding of how C++ classes work you should be able to add as many member functions as you like to OOData (so long as they are not virtual) and be able to use a Data* as an OOData instance using static_cast. However you won't be able to add member variables to OOData as this affects the size of OOData. But I'm sure you knew all this before!

If you really want to have OOData as a subclass of Data and have extra data members you might be able to play a few tricks by redefining OOData's delete operator. The trick would be for delete to somehow differentiate between a real OOData instance and one which was really just a Data instance (without OOData's members) and do the appropriate freeing up of memory. Perhaps there is some member in Data that could be used in a dual role to signify the difference?

But I think the best solution is to use composition as others have suggested.

ß e n

EDIT: Oh, the other thing that may be of use to you is the placement new() operator. It might be a better way of creating an OOData object out of an existing Data pointer!

 

[zmttoxics]

I would implement a struct Data* or void* and in your constructor take in and assign it like what was mentioned above.

private:
Data * structData;

public:
OOData(Data * data){
structData = data;
}

Nothing wrong with attaching it to the class either.

 

[Analog Kid]

EDIT: Oh, the other thing that may be of use to you is the placement new() operator. It might be a better way of creating an OOData object out of an existing Data pointer!

Thanks! That's exactly the answer I was looking for originally but after thinking about the implications I decided that wasn't such a good idea after all.

Here's what I've settled on for the time being:

typedef struct Data 
{
 int x; 
 int bigArrayType; 
 char * bigArray;
} Data;

Same legacy data structure as before. Thinking it through, I realize there are really two cases-- where I'm effectively doing a copy of the original data structure, and where I'm using legacy factory methods to create a new data structure. I broke that into two cases:

class OOData: public Data
{
private:
  [COLOR="DimGray"]//handle the case where I'm converting a Data to an OOData by 
  //keeping the contents of bigArray and deleting the top level structure[/COLOR] 
  inline void absorbDataPtr(Data* d){*(Data*)this=*d;shallowRelease(d);};

  [COLOR="DimGray"]//handle the case where I'm copying a Data by doing a deep copy
  //creating another Data, which I then convert[/COLOR]
  inline void copyDataPtr(Data* d){absorbDataPtr(cloneData(d));};

  [COLOR="DimGray"]//prevent inadvertent copying by keeping these private[/COLOR]
  inline OOData(const OOData& d){copyDataPtr((Data*)d);};
  inline OOData& operator=(const OOData& d){copyDataPtr((Data*)d);return *this;};

public:
  OOData(Data* d){copyDataPtr(d);};
  OOData(int x, int type){absorbDataPtr(createData(x,type));};

  [COLOR="DimGray"]//intentional copy[/COLOR]
  OOData* copy(){return new OOData(*this);};
};

cloneData, shallowRelease and createData are existing legacy functions.

I think this strikes the right balance between performance, safety and not touching (or duplicating) legacy code. There's a performance hit on construction, minimal or no performance hit on use, I minimize unnecessary data allocations within the above guidelines and I don't think I ever do an unnecessary copy of bigArray.

Thanks to everyone for their input! Let me know if you see anything obviously wrong with the above.

 

[Sander]

Finding code like this in our archives would definitely have me raise an eyebrow. Casting "this" to something else doesn't look very kosher to me. If someone else later adds virtuals to this class, prepare for some ugly effects.

Also, it's not quite clear to me yet why you're wrapping the data type in the first place. Your OOData class doesn't seem to add much (but perhaps you left that away in your posting). If the only reason is "at least now it's a class" then you're doing overzealous OO