Add member to existing struct without breaking legacy code

Question

There is the following definition in some legacy code that I am working with.

struct VlanData
{
    uint16_t mEtherType;
    uint16_t mVlanId;
};

I want to add a new member to this struct:

struct VlanData
{
    uint16_t mEtherType;
    uint16_t mVlanId;
    uint8_t mVlanPriority; // New member
};

However, use of VlanData has been fairly inconsistent through the legacy code.

Not initialized on construction:

VlanData myVlans;
myVlans.mEtherType = 0x8100;
myVlans.mVlanId = 100;

Value initialized:

VlanData myVlans = { 0x8100, 100 };

What I want to do is come up with a safe way to ensure that `mVlanPriority' is automatically set to 0 in legacy code without updating a lot of code.

I understand I could modify the legacy code to value-initialize all members like so:

VlanData myVlans = {};

But I don't want to have to update all of this code. I believe that creating a default constructor like this would help:

VlanData()
: mEtherType(0),
  mVlanId(0),
  mVlanPriority(0)
{}

But this would also destroy the POD-ness of the struct.

So I guess I have several questions:

Is there a safe way I can ensure that mVlanPriority is set to 0 in legacy code without updating the legacy code?
What use of the class would break if this was no longer a POD type?

Adding a constructor will invalidate all those bracket initializers. — Tugrul Ates, Jul 19 '11 at 06:05
@junjanes - you mean code using bracket initializers wouldn't compile anymore? — LeopardSkinPillBoxHat, Jul 19 '11 at 06:06

Murali VP · Accepted Answer · 2011-07-19T07:01:00.367

5

struct VlanData {
    struct P
    {
        uint8_t operator=(uint8_t v) { mVlanPriority = v; }
        uint8_t mVlanPriority; P() { mVlanPriority = 0; };
    };
    uint16_t mEtherType;
    uint16_t mVlanId;
    P mVlanPriority;
 };

Define other kinds of operators and add type conversion functions as you need.

eg:

int main(int argc, char** argv)
{
    VlanData myVlans0 = { };
    VlanData myVlans = { 0x8100, 100 };
    myVlans.mVlanPriority = 10;
}

edited Jul 19 '11 at 07:01

answered Jul 19 '11 at 06:50

Murali VP

6,198
4
28
36

Is VLanData still Pod with this? If VlanData is used in a union, or is reinterpret_cast in other parts of the code, will those uses not then break? – Tom Jul 19 '11 at 07:03
@Tom: it's still POD. For a class to be non-POD it should have a non-POD instance field, reference instance field, user-defined destructor or user-defined copy assignment operator. Note that 'copy assignment operator' is not the same as 'assignment operator' defined in VlanData.P, so it's POD. – Konstantin Oznobihin Jul 19 '11 at 07:15
Hmm... VlanData has a data member of a struct that is not a POD, does that make VLanData not a POD? Given that mVlanPriority is a new member, the code where it is going to get used is new code anyway. Why wouldn't prior reinterpret_cast not continue to work? the entire code base has to be compiled anyway. Anywhere VlanData is used as a union, if there is any effect it should be no different from the effect of adding mVlanPriority as a uint8_t type. – Murali VP Jul 19 '11 at 07:15
@Tom: VlanData still can be considered a POD in this case. You can copy one with `memcpy(dst, &vlandata, sizeof(VlanData))` and the order of the fields in the first bytes haven't changed. – Tugrul Ates Jul 19 '11 at 07:17
+1, cool, very interesting solution, I didn't know you could do that. My concern was that with those extra functions you loose all guarentees as to the alignment of the class - but that was just me guessing – Tom Jul 19 '11 at 07:17
1

This version of `VlanData` definitely is not POD, it has a non-POD member. `P` is non-POD since it has a constructor. It may well be that on your implementation, it can be copied with `memcpy` successfully, but the same is just as likely to be true of the questioner's version that has a constructor. – Steve Jessop Jul 19 '11 at 08:54
It's not a POD, per Murali VP's comment. `VlanData::P` obviously isn't, so `VlanData` has a non-POD member and isn't a POD itself. POD is really just another name for "C-style structure" and C doesn't have structures with members initialized to zero. – MSalters Jul 19 '11 at 08:55

iammilind · Answer 2 · 2011-07-19T06:37:57.143

Is there a safe way I can ensure that mVlanPriority is set to 0 in legacy code without updating the legacy code?

No there is no standard way in current standard. You must have to have constructor.

What use of the class would break if this was no longer a POD type?

As @junjanes has mentioned in the comments, your code will be broken where you try to initialize the members with brackets.

Edit: To address your problem, I would suggest

struct VlanData
{
  uint16_t mEtherType;
  uint16_t mVlanId;
  uint8_t mVlanPriority; // New member

  VlanData(uint16_t ether, uint16_t id, uint8_t priority = 0) :
           mEtherType(ether), mVlanId(id), mVlanPriority(priority)
  {}
};

So now, your new variable will be initialized to 0 and you have to do a very less typing for fixing compilation error.

Change,

VlanData myVlans = { 0x8100, 100 };

To,

VlanData myVlans( 0x8100, 100 );

Tom · Answer 3 · 2011-07-19T07:14:20.813

I'm not an expert on c++0x, but I do know that the strict pod-ness guarentees were relaxed in c++0x, with the introduction of a standard-layout class. Your class with a constructor isn't pod but I believe it is standard-layout and so it might be worth checking your compilers compatability with this aspect of the new standard. I think the problems that you are having were fixed up quite a lot with c++0x.

I also believe that curly bracket initialization of standard layout classes is also allowed in c++0x. See the Initializer lists and Uniform initialization section of this wikipedia article.

Looking up what the standard-layout class fixes should give a fairly good list as to what can break for non-pod types in the current standard. For instance, reinterpret_cast is not safe in c++03 for non-pod types (the alignment might not be correct) but is safe in c++0x for standard-layout classes.

In short, I think your frustrations are well recognised and perhaps even ironed out in the new standard, but I don't think its going to possible to fix them all with the current standard.

My approach would be to try an leave legacy code untouched and slowly migrate to the new version of your class:

namespace version_1_1
{
  struct VlanData
  {
      uint16_t mEtherType;
      uint16_t mVlanId;
      uint8_t mVlanPriority; // New member
  };

  vlanData 
  convert_VlanData( ::VlanData const& v)
  {
     VlanData v2 = {v.mEtherType,v.mVlanId, 0};
     return v2;
  }
}

and then you are explicit and clear as to when you are using what - and migrate things when you need to.

Add member to existing struct without breaking legacy code

Not initialized on construction:

Value initialized:

3 Answers3