Skip to content

Use Padding and make public padding fields private - #5444

Merged
tgross35 merged 1 commit into
rust-lang:mainfrom
dybucc:add-padding-remaining-public
Sep 8, 2026
Merged

tgross35 merged 1 commit into
rust-lang:mainfrom
dybucc:add-padding-remaining-public

Conversation

@dybucc

@dybucc dybucc commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Description

Stems from #5435, but contains only the parts of the
patchset that affect currently public record fields.

Checklist

  • Relevant tests in libc-test/semver have been updated
  • Commit messages permalink to headers for added or changed API
  • Placeholder or unstable values like *LAST or *MAX have the
    standard doc comment
  • Tested locally (cargo test -p libc-test --target mytarget);
    especially relevant for platforms that may not be checked in CI

@rustbot

rustbot commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred in an OpenBSD module

cc @semarie

Some changes occurred in a solarish module

cc @jclulow, @pfmooney

Some changes occurred in a NetBSD-like module

cc @semarie

Some changes occurred in an Android module

cc @maurer

@rustbot

This comment has been minimized.

@tgross35 tgross35 changed the title crate: use Padding and make private public padding fields Use Padding and make private public padding fields Aug 31, 2026
@rustbot

This comment has been minimized.

@tgross35 tgross35 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tgross35 tgross35 added the stable-declined This change is breaking, difficult to backport, low priority, or otherwise not relevant for 0.2 label Aug 31, 2026
@tgross35

Copy link
Copy Markdown
Member

@rustbot author since that's in the queue not and there's a conflict

@rustbot

rustbot commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@dybucc

dybucc commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

@rustbot ready

@rustbot

This comment has been minimized.

@rustbot

This comment has been minimized.

@tgross35 tgross35 changed the title Use Padding and make private public padding fields Use Padding and make public padding fields private Sep 3, 2026

@tgross35 tgross35 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good but needs a rebase since it is, of course, conflict-y

@rustbot author

View changes since this review

pub uc_mcontext: mcontext_t,
#[cfg(target_os = "illumos")]
pub uc_brand_data: [*mut c_void; 3],
uc_filler3: Padding<[c_long; 3]>,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you mention this correction in the commit message?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

- Change public padding fields to be both private and use the `Padding`
  type. This has already been the norm for some time now, but there was
  still some cruft left. This patch finishes up the work, though this
  comes at the cost of breaking changes.

- Change `uc_brand_data` field in Illumos to use `c_long` instead of
  `*mut c_void` as padding type. See [^1]. The name of the field also
  changed but that is irrelevant as it was already private before this
  patch.

[^1]: <https://github.com/illumos/illumos-gate/blob/de1199e40761fcb5ed5cf82b16414ce4e4840999/usr/src/uts/intel/sys/ucontext.h#L89-L95>
@dybucc
dybucc force-pushed the add-padding-remaining-public branch from 1373c63 to 382d959 Compare September 7, 2026 08:51
@rustbot

rustbot commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@xtqqczze

xtqqczze commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

@tgross35 Since some of these changes are necessary to prevent Undefined Behavior, as reported in bytecodealliance/rustix#1668 should they be considered for backporting?

@xtqqczze

xtqqczze commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

@dybucc Once this is merged, I think making deprecated fields use Padding, along with removing pub from these fields, should be considered. For example: __st_ino, __old_st_blksize, and __old_st_*.

@tgross35

tgross35 commented Sep 8, 2026

Copy link
Copy Markdown
Member

@tgross35 Since some of these changes are necessary to prevent Undefined Behavior, as reported in bytecodealliance/rustix#1668 should they be considered for backporting?

Not backporting, bug in rustix bytecodealliance/rustix#1668 (comment)

@tgross35 tgross35 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tgross35
tgross35 added this pull request to the merge queue Sep 8, 2026
Merged via the queue into rust-lang:main with commit eb975ff Sep 8, 2026
58 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants