Skip to content

core: add deserialize_fd() helper + other minor fixes - #29481

Merged
bluca merged 3 commits into
systemd:mainfrom
poettering:deserialize-fd
Oct 18, 2023
Merged

bluca merged 3 commits into
systemd:mainfrom
poettering:deserialize-fd

Conversation

@poettering

Copy link
Copy Markdown
Member

No description provided.

@poettering poettering added the pid1 label Oct 6, 2023
@github-actions github-actions Bot added util-lib tests please-review PR is ready for (re-)review by a maintainer labels Oct 6, 2023
Comment thread src/core/automount.c
Comment thread src/core/dynamic-user.c Outdated
Comment thread src/core/execute.c
Comment thread src/shared/serialize.c Outdated
Comment thread src/core/service.c Outdated
@poettering

Copy link
Copy Markdown
Member Author

Force pushed new version, with requested changes made.

@bluca

This comment was marked as resolved.

@poettering
poettering force-pushed the deserialize-fd branch 2 times, most recently from 79432cf to 986d1d9 Compare October 11, 2023 13:56
bluca

This comment was marked as resolved.

@bluca bluca added needs-rebase good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed please-review PR is ready for (re-)review by a maintainer labels Oct 13, 2023
@bluca

bluca commented Oct 16, 2023

Copy link
Copy Markdown
Member

15:43:11 [ 419.191559] testsuite-23.sh[53]: Subtest /usr/lib/systemd/tests/testdata/units/testsuite-23.ExecStopPost.sh failed

@bluca bluca added ci-fails/needs-rework 🔥 Please rework this, the CI noticed an issue with the PR and removed good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed labels Oct 16, 2023
@github-actions github-actions Bot added tests please-review PR is ready for (re-)review by a maintainer labels Oct 17, 2023
@github-actions github-actions Bot removed the ci-fails/needs-rework 🔥 Please rework this, the CI noticed an issue with the PR label Oct 17, 2023
@bluca bluca added ci-fails/needs-rework 🔥 Please rework this, the CI noticed an issue with the PR and removed please-review PR is ready for (re-)review by a maintainer labels Oct 17, 2023
Comment thread src/core/service.c Outdated
Comment thread src/core/execute.c
Comment thread src/core/execute.c
Comment thread src/core/execute-serialize.c Outdated
@github-actions github-actions Bot added please-review PR is ready for (re-)review by a maintainer and removed ci-fails/needs-rework 🔥 Please rework this, the CI noticed an issue with the PR labels Oct 18, 2023
Comment thread src/core/execute-serialize.c
Comment thread src/core/execute.c Outdated
Currently, when we deserialize an fd we do a lot of manual work. Add a
common helper that makes this more robust and uniform.

Note that this sometimes changes behaviour slightly, but in ways that
shouldn't really matter: if we fail to deserialize an fd correctly we'll
unset (i.e. set to -EBADF) the fd in the deserialized data structure.
Previously, we'd leave the old value in place.

This should not change effective result (as in either case we'll be in a
bad state afterwards, just once we mix old/invalidated state with new
state, while now we'll reset the state explicitly to invalidated state
on failure). In particular as deserialization starts from an empty
structure generally, hence the old value should be unset anyway.

Another slight change is that if we fail to deserialize some object half
way, and we already have taken out one fd from the serialized fdset
we'll now just close it instead of returning it to/leaving it in the
fdset. Given that such "orphaned" fds are blanket closed after
deserialization finishes this also shouldn't change behaviour IRL.

Also, the idle_pipe was previously incorrectly serialized: we'd
serialize invalidated fds, which would fail, but because parsing errors
on this were ignored on the deserializatin noone noticed. This is fixed.
Rename the return parameters "ret", and use compound initialization. Add
an assert() on input.
The other deserializers put value first, and return parameter second,
let's do so here too.
@bluca bluca added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed tests please-review PR is ready for (re-)review by a maintainer labels Oct 18, 2023
@bluca
bluca merged commit c2e42d4 into systemd:main Oct 18, 2023
@bluca bluca added replaced-by-newer-pr and removed good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed labels Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants