WIP: Introduce `LTC_NO_STRUCT_PADDING`. #757

Draft

sjaeckel wants to merge 1 commit from no-struct-padding into develop

sjaeckel commented

2026-05-04 08:34:50 -05:00

(Migrated from github.com)

Pulled out from #751

https://github.com/libtom/libtomcrypt/pull/751#issuecomment-4365899260

The only exception is the LTC_NO_STRUCT_PADDING part, which I would suggest moving into a separate PR and perhars discuss pros/cons there. I have doubts whether it will work as expected, especially the change from ulong64 dummy[8] to void *padding[8]. In my opinion, this goes in the wrong direction as the reserved area becomes platform-dependent: 64 bytes vs. 32 bytes, depending on the pointer size.

Pulled out from #751 https://github.com/libtom/libtomcrypt/pull/751#issuecomment-4365899260 > The only exception is the LTC_NO_STRUCT_PADDING part, which I would suggest moving into a separate PR and perhars discuss pros/cons there. I have doubts whether it will work as expected, especially the change from ulong64 dummy[8] to void *padding[8]. In my opinion, this goes in the wrong direction as the reserved area becomes platform-dependent: 64 bytes vs. 32 bytes, depending on the pointer size.

karel-m (Migrated from github.com) reviewed

2026-05-04 08:34:50 -05:00

sjaeckel commented

2026-05-05 03:35:33 -05:00

(Migrated from github.com)

I have doubts whether it will work as expected, especially the change from ulong64 dummy[8] to void *padding[8]. In my opinion, this goes in the wrong direction as the reserved area becomes platform-dependent: 64 bytes vs. 32 bytes, depending on the pointer size.

That's indeed going to become platform-dependent, but struct sizes always are as soon as they contain platform-dependent types like int or pointer types.

pahole says this:

                                                                64bit      |        32bit
                                                                           |
struct ltc_rsa_op_parameters {                                             | 
        ltc_rsa_parameters         params;               /*     0    16 */ | /*     0    12 */
        int                        padding;              /*    16     4 */ | /*    12     4 */
        int                        wprng;                /*    20     4 */ | /*    16     4 */
        prng_state *               prng;                 /*    24     8 */ | /*    20     4 */
        union {                                                            | 
                struct {                                                   | 
                        const unsigned char  * lparam;   /*    32     8 */ | /*    24     4 */
                        long unsigned int lparamlen;     /*    40     8 */ | /*    28     4 */
                } crypt;                                 /*    32    16 */ | /*    24     8 */
                void *             padding[8];           /*    32    64 */ | /*    24    32 */
        } u;                                             /*    32    64 */ | /*    24    32 */
                                                                           |
                                 /* size: 96, cachelines: 2, members: 5 */ | /* size: 56, cachelines: 1, members: 5 */
                                 /* last cacheline: 32 bytes */            | /* last cacheline: 56 bytes */
};

The idea was to pad the struct up to an even size, e.g. a multiple of 32 bytes, and currently I only did that for x86_64, so we should
a) decide whether this makes sense or whether we should simply pad with some members of ulong64
and if we want to pad up to a specific size
b) decide what value to pad up to
c) do this for 32bit archs as well

Does that make sense?

> I have doubts whether it will work as expected, especially the change from ulong64 dummy[8] to void *padding[8]. In my opinion, this goes in the wrong direction as the reserved area becomes platform-dependent: 64 bytes vs. 32 bytes, depending on the pointer size. That's indeed going to become platform-dependent, but struct sizes always are as soon as they contain platform-dependent types like `int` or pointer types. `pahole` says this: ``` 64bit | 32bit | struct ltc_rsa_op_parameters { | ltc_rsa_parameters params; /* 0 16 */ | /* 0 12 */ int padding; /* 16 4 */ | /* 12 4 */ int wprng; /* 20 4 */ | /* 16 4 */ prng_state * prng; /* 24 8 */ | /* 20 4 */ union { | struct { | const unsigned char * lparam; /* 32 8 */ | /* 24 4 */ long unsigned int lparamlen; /* 40 8 */ | /* 28 4 */ } crypt; /* 32 16 */ | /* 24 8 */ void * padding[8]; /* 32 64 */ | /* 24 32 */ } u; /* 32 64 */ | /* 24 32 */ | /* size: 96, cachelines: 2, members: 5 */ | /* size: 56, cachelines: 1, members: 5 */ /* last cacheline: 32 bytes */ | /* last cacheline: 56 bytes */ }; ``` The idea was to pad the struct up to an even size, e.g. a multiple of 32 bytes, and currently I only did that for x86_64, so we should a) decide whether this makes sense or whether we should simply pad with some members of ulong64 and if we want to pad up to a specific size b) decide what value to pad up to c) do this for 32bit archs as well Does that make sense?

karel-m commented

2026-07-30 10:06:00 -05:00

(Migrated from github.com)

You are probably waiting for feedback from me, but frankly, API compatibility is not something I care much about. Fortunately, I do not have to deal with it, so my expertise in this area is limited.

I plan to use LTC_NO_STRUCT_PADDING, but I do not have a strong opinion on how best to handle structure padding to avoid future API breakage. The approach chosen in this PR is fine with me.

You are probably waiting for feedback from me, but frankly, API compatibility is not something I care much about. Fortunately, I do not have to deal with it, so my expertise in this area is limited. I plan to use `LTC_NO_STRUCT_PADDING`, but I do not have a strong opinion on how best to handle structure padding to avoid future API breakage. The approach chosen in this PR is fine with me.

This pull request is marked as a work in progress.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin 'no-struct-padding:no-struct-padding'

git switch 'no-struct-padding'

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch 'develop'

git merge --no-ff 'no-struct-padding'

git switch 'no-struct-padding'

git rebase 'develop'

git switch 'develop'

git merge --ff-only 'no-struct-padding'

git switch 'no-struct-padding'

git rebase 'develop'

git switch 'develop'

git merge --no-ff 'no-struct-padding'

git switch 'develop'

git merge --squash 'no-struct-padding'