Page MenuHome

makesdna: fix detecting 32 bit padding issues.
ClosedPublic

Authored by Ray Molenkamp (LazyDodo) on Aug 10 2021, 11:19 PM.

Details

Summary

makesdna fails to detect issues in 32 bit code that can
only be resolved by adding a padding pointer.

We never noticed since we ourselves no longer build for
32 bit, but debian's 32 bit builds got bitten by this

A rather extensive explanation on why this is alignment
requirement is there can be found in this comment:

https://developer.blender.org/D9389#233034


I have to admit, I do not enjoy working on makesdna and I'm
honestly terrified I broke it, review this very very critically

This is currently throwing out *LOTS* of errors that are
false positives (ie ModifierData need a padding field,
but it flags any structs that have ModifierData in it
as well) then again, that's just what makesdna seems to do
whenever it gets unhappy, so no change there.

D9389 needs to land before this can land.

Diff Detail

Repository
rB Blender
Branch
tmp_makesdna_32bit_detect_fix (branched from master)
Build Status
Buildable 16348
Build 16348: arc lint + arc unit

Event Timeline

Ray Molenkamp (LazyDodo) requested review of this revision.Aug 10 2021, 11:19 PM
Ray Molenkamp (LazyDodo) created this revision.
Ray Molenkamp (LazyDodo) added a project: Core.

I winged the reviewers a little bit based on makesdna.c's commit history, if anyone else wants to jump in, more eyeballs are always welcome..

LGTM, double checked this works with small structs from recent rBa0867f05a48e2017a3b634cda5471c015af5bf35 commit.

Minor issues noted inline.

Accepting, as an extra iteration from me isn't needed.

source/blender/makesdna/intern/makesdna.c
976

*picky* use capitals.

980

This is no longer read, can be removed.

1056–1057

max isn't defined for GCC, use MAX2

This revision is now accepted and ready to land.Aug 11 2021, 2:46 AM

Overall fine, but please don't forget to do max->MAX2 change before commit ;)

source/blender/makesdna/intern/makesdna.c
1056–1057

To me the code does not compile without this change.