Prevent misuse of structures designed solely for transport

281 Views Asked by At

I work on a product which has multiple components, most of them are written in C++ but one is written in C. We often have scenarios where a piece of information flows through each of the components via IPC.

We define these messages using structs so we can pack them into messages and send them over a message queue. These structs are designed for 'transport' purposes only and are written in a way that serves only that purpose. The problem I'm running into is this: programmers are holding onto the struct and using it as a long-term container for the information.

In my eyes this is a problem because:

1) If we change the transport structure, all of their code is broken. There should be encapsulation here so that we don't run into this scenario.

2) The message structs are very awkward and are designed only to transport the information...It seems highly unlikely that this struct would also happen to be the most convenient form of accessing this data (long term) for these other components.

My question is: How can I programmatically prevent this mis-usage? I'd like to enforce that these structures are only able to be used for transport.

EDIT: I'll try to provide an example here the best I can:

struct smallElement {
    int id;
    int basicFoo;
};

struct mediumElement {
    int id;
    int basicBar;
    int numSmallElements;
    struct smallElement smallElements[MAX_NUM_SMALL];
};

struct largeElement {
    int id;
    int basicBaz;
    int numMediumElements;
    struct mediumElement[MAX_NUM_MEDIUM];
};

The effect is that people just hold on to 'largeElement' rather than extracting the data they need from largeElement and putting it into a class which meets their needs.

5

There are 5 best solutions below

0
On

i can't say it more simple than this: use protocol buffers. it will make your life so much easier in the long run.

0
On

You could implement them in terms of const references so that server side constructs the transport struct but client usage is only allowed to have const references to them and can't actually instantiate or construct them to enforce the usage you want.

Unfortunately without a code snippets of your messages, packaging, correct usage, and incorrect usage I can't really provide more detail on how to implement this in your situation but we use something similar in our data model to prevent improper usage. I also export and provide template storage classes to ease the population from the message for client usage when they do want to store the retrieved data.

2
On

When I define message structures (in C++, not valid in C ) I make sure that :

  1. the message object is copiable
  2. the message object have to be built only once
  3. the message object can't be changed after construction

I'm not sure if the messages will still be pods but I guess it's equivalent from the memory point of view.

The things to do to achieve this :

  1. Have one unique constructor that setup every members
  2. have all memebers private
  3. have const member accessors

For example you could have this :

struct Message
{
   int id;
   long data;
   Info info;
};

Then you should have this :

class Message // struct or whatever, just make sure public/private are correctly set
{
public:
   Message( int id, long data, long info ) : m_id( id ), m_data( data ), m_info( info ) {}

   int id() const { return m_id; }
   long data() const { return m_data; }
   Info info() const { return m_info; }

private:

   int m_id;
   long m_data;
   Info m_info;

};

Now, the users will be able to build the message, to read from it, but not change it in the long way, making it unusable as a data container. They could however store one message but will not be able to change it later so it's only useful for memory.


OR.... You could use a "black box".

  1. Separate the message layer in a library, if not already.
  2. The client code shouldn't be exposed at all to the message struct definitions. So don't provide the headers, or hide them or something.
  3. Now, provide functions to send the messages, that will (inside) build the messages and send them. That will even help with reading the code.
  4. When receiving messages, provide a way to notify the client code. But don't provide the pessages directly!!! Just keep them somewhere (maybe temporarly or using a lifetime rule or something) inside your library, maybe in a kind of manager, whatever, but do keep them INSIDE THE BLACK BOX. Just provide a kind of message identifier.
  5. Provide functions to get informations from the messages without exposing the struct. To achieve this you have several ways. I would be in this case, I would provide functions gathered in a namespace. Those functions would ask the message identifier as first parameter and will return only one data from the message (that could be a full object if necessary).

That way, the users just cant use the structs as data containers, because they dont have their definitions. they conly can access the data.

There a two problems with this : obvious performance cost and it's clearly heavier to write and change. Maybe using some code generator would be better. Google Protobuf is full of good ideas in this domain.


But the best way would be to make them understand why their way of doing will break soon or later.

0
On

usually there is a bad idea to define transport messages as structures. It's better to define "normal" (useful for programmer) struct and serializer/deserializer for it. To automate serializer/deserializer coding it's possible to define structure with macroses in a separate file and generate typedef struct and serializer/deserializer automatically(boost preprocessor llibrary may also help)

1
On

The programmers are doing this because its the easiest path of least resistance to getting the functionality they want. It may be easier for them to access the data if it were in a class with proper accessors, but then they'd have to write that class and write conversion functions.

Take advantage of their laziness and make the easiest path for them be to do the right thing. For each message struct you creat, create a corresponding class for storing and accessing the data using a nice interface with conversion methods to make it a one liner for them to put the message into the class. Since the class would have nicer accessor methods, it would be easier for them to use it than to do the wrong thing. eg:

msg_struct inputStruct = receiveMsg();
MsgClass msg(inputStruct);
msg.doSomething()
...
msg_struct outputStruct = msg.toStruct();

Rather than find ways to force them to not take the easy way out, make the way you want them to use the code the easiest way. The fact that multiple programmers are using this antipattern, makes me think there is a piece missing to the library that should be provided by the library to accomodate this. You are pushing the creation of this necessary component back on the users of the code, and not likeing the solutions they come up with.