6

My C library, in version 1.0.0, defines a struct and some functions to allocate and use the struct:

typedef struct { int x; int y; } MyStruct;
MyStruct *allocate( int, int );
void destroy( MyStruct* );
void print( MyStruct* );

Users are supposed to never allocate the struct themselves nor to copy it by value. this is the main difference from the question Semantic versioning: minor or major change?. For instance a program should use it like this:

void f(){
  MyStruct *ms = allocate(0,0);
  ms->x += 1;
  print(ms);
  destroy(ms);
}

Now I need to add a new field to the struct, while the signature of the functions doesn't change.

typedef struct { int x; int y; int z; } MyStruct;

The new struct takes more memory than the old one: if a program tries to allocate a MyStruct instance directly or to copy it by value it might break, if it's linked against a version of the library different from the one it was built with.

However that's not how programs use MyStruct: as long as they follow the documentation everything work fine. But nothing in the code prevents them from misusing the struct.

I'm using Semantic Versioning to version my library. In the case above should I increase the minor version (functionality added in a backwards compatible manner) or the major one (incompatible API changes)?

Blue Nebula
  • 932
  • 4
  • 9
  • 3
    IMO, this is not an incompatible change. Any program that uses the struct directly instead of the accessor functions is bypassing the API and shooting themselves in the foot. You are not responsible for their mistake. – William Pursell Feb 25 '21 at 17:08
  • 2
    It would be better not to completely define the `MyStruct` type at all in the API headers, and only completely define it within the library implementation. That would not affect legitimate uses of `MyStruct *` by the library user. To do this, you would need a tag name for the incomplete `struct` type, but that is not really a backwards incompatible change either (except for users who broke the rules). – Ian Abbott Feb 25 '21 at 17:18
  • 1
    @IanAbbott: but if I do that, I need a getter and a setter for every field. That's uncomfortable to type, unnecessary and would take more memory and execution time... It's a high price just to enforce proper usage. – Blue Nebula Feb 25 '21 at 17:22
  • I never noticed your `void f()` function was accessing the members directly. I think that makes the code change borderline compatible at best. I hope you never change the order or type of the members between the start of the structure and the last member that the library user is allowed to access directly. – Ian Abbott Feb 25 '21 at 17:28
  • 1
    Right, renaming fields or changing their order or type would be an incompatible API change. – Blue Nebula Feb 25 '21 at 17:30
  • 2
    One thing you could do is embed `MyStruct` in a larger structure private to the library implementation and put the extra stuff in the larger structure. That only works for extra stuff that you don't intend the user to access directly. – Ian Abbott Feb 25 '21 at 17:34
  • 1
    Without the documentation, it's not entirely clear whether you've got a minor or major version change there, but given that this is C code, and we don't have visibility into the documentation, I would err on the side of safety, and say you definitely have a breaking change there. The size of `MyStruct` has changed, that breaks everything that depends on that structure. This goes beyond how the structure is allocated. What about storage? – jwdonahue Feb 25 '21 at 18:48
  • 1
    "*Users are supposed to never allocate the struct themselves*" But that's not enforced, so users could technically create local (not heap) based instances, or an array of `MyStruct`, or serialize it based on `sizeof`. In such cases, changing the layout and/or size of the structure can introduce obscure, hard to find bugs. For your own good, consider this to be a major, breaking change. – dxiv Feb 25 '21 at 19:13

1 Answers1

7

You made a breaking change to a publicly visible structure, that in C, is not backwards compatible with the earlier version. Beside the change in structure size, your own example demonstrates exactly why this is a breaking change:

void f(){
  MyStruct *ms = allocate(0,0);
  ms->x += 1;
  print(ms);   // Indicates you are aware of external dependencies.
  destroy(ms);
}

You are exposing a data structure, that may be incorporated by your customer's, into a data set of some kind. You may have some ideas about how your library will be used, but I can assure you that your customers will always surprise you.

Given the code you posted, and your stated intent wrt how your library is to be used, I'd say you need to redesign your API, such that MyStruct is entirely opaque to the user and never changes size (customer's often cache things like this). Borrow a page from the standard library, use a handle.

typedef int MyHandle;
MyHandle allocate(int x, int y);
void destroy(MyHandle h);
void print(MyHandle h);

The handle can be range checked by your internal code and then used as index into a table of struct or struct pointers, or it could be key into binary tree. The point is you are free do whatever you like, without disrupting your API.

If you intend for the x y bits to be visible, use an extendable struct:

typedef struct { 
    int x; 
    int y; 
    void* reserved; // For internal use only!
} MyStruct;

The MyStruct.reserved field should always be NULL until you need it for internal use. Just keep in mind, that once you expose data fields like this, how they are used by your customers, is completely out of your hands. When you expose structure in this way, you are making a commitment to your customers.

Regarding getters and setters.

// Using the MyHandle type described earlier:
typedef struct _My_XY {
    int x;
    int y;
} My_XY;

My_XY GetXY(MyHandle h);

Problem solved.

I will add that since adding the z field is a breaking change anyway, and you seem to feel the need to extend the functionality of your product, this would be an opportune time for a radical API rewrite. But if you must extend it without breaking your customer base, you can hide the z data point internally with something like:

// Public API
typedef struct { int x; int y; } MyStruct;
MyStruct *allocate( int, int );
void destroy( MyStruct* );
void print( MyStruct* );

// Implementation

typedef struct _XY_Node {
    MyStruct* xy;
    int z;
    struct _XY_Node *pNext;
} XY_Node;

XY_Node root = null;

XY_Node* AddNode(int x, int y, int z) {...}
XY_Node* RemoveNode(int x, int y, int z) {...}
XY_Node* FindNode(int x, int y, int z) {...}

int DeriveZ(int x, int y) {...}

MyStruct *allocate(int x, int y)
{
    return AddNode(x, y, DeriveZ(x, y)).xy;
}

// etc...

You can use a hash table, or some form of binary tree, rather than list. The point is, you don't have to break your API in order to extend it. You could do this as a final release in the 1.y.z series, and then completely overhaul your API, such that you have a cleaner customer experience, and a more efficient implementation.

jwdonahue
  • 6,199
  • 2
  • 21
  • 43