26/01/2022 11:52, Bruce Richardson: > The scenario is as follows. Suppose we have the initial state as below: > > struct x_dev_cfg { > int x; > }; > > int > x_dev_cfg(int dev_id, struct x_dev_cfg *cfg) > { > struct x_dev *dev = x_devs[id]; > // some setup/config may go here > return dev->configure(cfg, sizeof(cfg)); // sizeof(cfg) == 4 > } > > Now, supposing we need to add in a new field into the config structure, a > very common occurance. This will indeed break the ABI, so we need to use > ABI versioning, to ensure that apps passing in the old structure, only call > a function which expects the old structure. Therefore, we need a copy of > the old structure, and a function to work on it. This gives this result: > > struct x_dev_cfg { > int x; > bool flag; // new field; > }; > > struct x_dev_cfg_v22 { // needed for ABI-versioned function > int x; > }; > > /* this function will only be called by *newly-linked* code, which uses > * the new structure */ > int > x_dev_cfg(int dev_id, struct x_dev_cfg *cfg) > { > struct x_dev *dev = x_devs[id]; > // some setup/config may go here > return dev->configure(cfg, sizeof(cfg)); // sizeof(cfg) is now 8 > } > > /* this function is called by apps linked against old version */ > int > x_dev_cfg_v22(int dev_id, struct x_dev_cfg_v22 *cfg) > { > struct x_dev *dev = x_devs[id]; > // some setup/config may go here > return dev->configure((void *)cfg, sizeof(cfg)); // sizeof(cfg) is still 4 > } > > With the above library code, we have different functions using the > different structures, so ABI compatibility is preserved - apps passing in a > 4-byte struct call a function using the 4-byte struct, while newer apps can > use the 8-byte version. > > The final part of the puzzle is then how drivers react to this change. > Originally, all drivers only use "x" in the config structure because that > is all that there is. That will still continue to work fine in the above > case, as both 4-byte and 8-byte structs have the same x value at the same > offset. i.e. no driver updates for x_dev is needed. > > On the other hand, if there are drivers that do want/need the new field, > they can also get to use it, but they do need to check for its presence > before they do so, i.e they would work as below: > > if (size_param > struct(x_dev_cfg_v22)) { // or "== struct(x_dev_cfg)" > // use flags field > } > > Hope this is clear now.
Yes, this is the kind of explanation we need in our guideline doc. Alternatives can be documented as well. If we can list pros/cons in the doc, it will be easier to choose the best approach and to explain the choice during code review.