Unifying boilerplate code for handling child processes - part 2 by alexey-tikhonov · Pull Request #8174 · SSSD/sssd

alexey-tikhonov · 2025-11-07T12:15:08Z

Follow up of #7991

alexey-tikhonov · 2025-11-13T19:06:52Z

Covscan complains

332    if (getresgid(&rgid, &egid, &sgid) == 0) {
     
CID 638588: (#1 of 1): Unchecked return value (CHECKED_RETURN)
18. check_return: Calling setresgid without checking return value (as is done elsewhere 4 out of 5 times).
333        setresgid(sgid, sgid, sgid);

but this PR doesn't touch this code. Weird.

No other issues reported.

alexey-tikhonov · 2025-11-14T08:14:10Z

(rebased)

alexey-tikhonov · 2025-11-14T17:35:44Z

Ok, at least this hang on

tests/test_identity.py::test_identity__lookup_group_gid_with_getent (ldap) FAILED [ 47%]

isn't introduced by this PR.

https://github.com/SSSD/sssd/actions/runs/19366604432/job/55412465094?pr=8193 has the same issue.

alexey-tikhonov · 2025-11-18T18:26:29Z

(rebased)

thalman

This looks great, thanks @alexey-tikhonov,

See my one nitpicking comment, sorry for that ;-)

thalman · 2025-11-19T12:48:38Z

src/util/child_bootstrap.h

+    SSSD_DEBUG_OPTS                                                        \
+    SSSD_LOGGER_OPTS(&sss_child_basic_settings.opt_logger)                 \
+    {"dumpable", 0, POPT_ARG_INT, &sss_child_basic_settings.dumpable, 0,   \
+     sss_child_basic_settings.ignore_dumpable ? _("Ignored") : _("Allow core dumps"), NULL },                                        \


Just nitpicking, can you remove some extra spaces before trailing \?

thalman · 2025-11-19T13:03:45Z

There was some github outage yesterday, This is why CI is so red.

thalman

Thanks, ACK

pbrezina

Re-running PR CI but ack. Nice job!

Reviewed-by: Pavel Březina <pbrezina@redhat.com> Reviewed-by: Tomáš Halman <thalman@redhat.com>

to 'sss_chain_id.h' where it makes a better sense. Reviewed-by: Pavel Březina <pbrezina@redhat.com> Reviewed-by: Tomáš Halman <thalman@redhat.com>

Since this header uses `gettext()` Reviewed-by: Pavel Březina <pbrezina@redhat.com> Reviewed-by: Tomáš Halman <thalman@redhat.com>

instead of DEBUG_CHAIN_ID_FMT_CID since this process is invoked by a provider. Reviewed-by: Pavel Březina <pbrezina@redhat.com> Reviewed-by: Tomáš Halman <thalman@redhat.com>

Plus an application in 'ldap_child'. Reviewed-by: Pavel Březina <pbrezina@redhat.com> Reviewed-by: Tomáš Halman <thalman@redhat.com>

Reviewed-by: Pavel Březina <pbrezina@redhat.com> Reviewed-by: Tomáš Halman <thalman@redhat.com>

This also makes 'passkey_child' to use '--chain-id' if supplied. Reviewed-by: Pavel Březina <pbrezina@redhat.com> Reviewed-by: Tomáš Halman <thalman@redhat.com>

Reviewed-by: Pavel Březina <pbrezina@redhat.com> Reviewed-by: Tomáš Halman <thalman@redhat.com>

This avoids using 'sssd' potentially by multiple binaries. Reviewed-by: Pavel Březina <pbrezina@redhat.com> Reviewed-by: Tomáš Halman <thalman@redhat.com>

Reviewed-by: Pavel Březina <pbrezina@redhat.com> Reviewed-by: Tomáš Halman <thalman@redhat.com>

sssd-bot · 2025-11-21T13:02:28Z

The pull request was accepted by @pbrezina with the following PR CI status:

🟢 CodeFactor (success)
🟢 CodeQL (success)
NEUTRAL osh-diff-scan:fedora-rawhide-x86_64:upstream (neutral)
🟢 rpm-build:centos-stream-10-x86_64:upstream (success)
🟢 rpm-build:fedora-41-x86_64:upstream (success)
🟢 rpm-build:fedora-42-x86_64:upstream (success)
🟢 rpm-build:fedora-43-x86_64:upstream (success)
🟢 rpm-build:fedora-rawhide-x86_64:upstream (success)
🟢 Analyze (target) / cppcheck (success)
🟢 Build / freebsd (success)
🟢 Build / make-distcheck (success)
🟢 ci / intgcheck (centos-10) (success)
🟢 ci / intgcheck (fedora-41) (success)
🟢 ci / intgcheck (fedora-42) (success)
🟢 ci / intgcheck (fedora-43) (success)
🟢 ci / intgcheck (fedora-44) (success)
🟢 ci / prepare (success)
🟡 ci / system (centos-10) (in_progress)
🟢 ci / system (fedora-41) (success)
🟢 ci / system (fedora-42) (success)
🟡 ci / system (fedora-43) (in_progress)
🟡 ci / system (fedora-44) (in_progress)
➖ Coverity scan / coverity (skipped)
🟢 Static code analysis / codeql (success)
🟢 Static code analysis / pre-commit (success)
🟢 Static code analysis / python-system-tests (success)

There are unsuccessful or unfinished checks. Make sure that the failures are not related to this pull request before merging.

alexey-tikhonov added the no-backport This should go to target branch only. label Nov 7, 2025

alexey-tikhonov force-pushed the child-helpers-2 branch 3 times, most recently from 8895fee to 337d467 Compare November 13, 2025 13:26

alexey-tikhonov marked this pull request as ready for review November 13, 2025 13:26

alexey-tikhonov requested a review from thalman November 13, 2025 13:43

alexey-tikhonov assigned thalman Nov 13, 2025

alexey-tikhonov requested a review from pbrezina November 13, 2025 13:43

alexey-tikhonov assigned pbrezina Nov 13, 2025

alexey-tikhonov added coverity Trigger a coverity scan and removed coverity Trigger a coverity scan labels Nov 13, 2025

alexey-tikhonov force-pushed the child-helpers-2 branch from 337d467 to 09b7586 Compare November 14, 2025 08:12

alexey-tikhonov mentioned this pull request Nov 15, 2025

Unifying boilerplate code for handling child processes - part 3 #8203

Merged

alexey-tikhonov force-pushed the child-helpers-2 branch from 09b7586 to 41ba181 Compare November 18, 2025 18:26

thalman requested changes Nov 19, 2025

View reviewed changes

thalman added the Changes requested label Nov 19, 2025

alexey-tikhonov force-pushed the child-helpers-2 branch from 41ba181 to ebdc990 Compare November 19, 2025 15:06

alexey-tikhonov added Waiting for review and removed Changes requested labels Nov 19, 2025

alexey-tikhonov requested a review from thalman November 19, 2025 15:07

thalman approved these changes Nov 20, 2025

View reviewed changes

pbrezina approved these changes Nov 21, 2025

View reviewed changes

pbrezina added Accepted and removed Waiting for review labels Nov 21, 2025

Get rid of useless SSSD_MAIN_OPTS define.

101c261

Reviewed-by: Pavel Březina <pbrezina@redhat.com> Reviewed-by: Tomáš Halman <thalman@redhat.com>

alexey-tikhonov added 16 commits November 21, 2025 13:02

Makefile: tools do not need to link against 'sss_client' code

7ea9518

Reviewed-by: Pavel Březina <pbrezina@redhat.com> Reviewed-by: Tomáš Halman <thalman@redhat.com>

Makefile: get rid of useless 'SSSD_LCL_TOOLS_OBJ'

482c8ad

Reviewed-by: Pavel Březina <pbrezina@redhat.com> Reviewed-by: Tomáš Halman <thalman@redhat.com>

Move 'DEBUG_CHAIN_ID_FMT_*' from 'util.h'

4418cae

to 'sss_chain_id.h' where it makes a better sense. Reviewed-by: Pavel Březina <pbrezina@redhat.com> Reviewed-by: Tomáš Halman <thalman@redhat.com>

Include <libintl.h> in 'debug.h'

b39bad3

Since this header uses `gettext()` Reviewed-by: Pavel Březina <pbrezina@redhat.com> Reviewed-by: Tomáš Halman <thalman@redhat.com>

OIDC_CHILD: use DEBUG_CHAIN_ID_FMT_RID

c89f363

instead of DEBUG_CHAIN_ID_FMT_CID since this process is invoked by a provider. Reviewed-by: Pavel Březina <pbrezina@redhat.com> Reviewed-by: Tomáš Halman <thalman@redhat.com>

Helpers to do a basic setup of a child process.

f64ec79

Plus an application in 'ldap_child'. Reviewed-by: Pavel Březina <pbrezina@redhat.com> Reviewed-by: Tomáš Halman <thalman@redhat.com>

KRB5_CHILD: use new helper to setup a process

05eee4a

Reviewed-by: Pavel Březina <pbrezina@redhat.com> Reviewed-by: Tomáš Halman <thalman@redhat.com>

OIDC_CHILD: use new helper to setup a process

3250cbe

Reviewed-by: Pavel Březina <pbrezina@redhat.com> Reviewed-by: Tomáš Halman <thalman@redhat.com>

AD_GPO_CHILD: use new helper to setup a process

ff85ab9

Reviewed-by: Pavel Březina <pbrezina@redhat.com> Reviewed-by: Tomáš Halman <thalman@redhat.com>

P11_CHILD: use new helper to setup a process

025ad2a

Reviewed-by: Pavel Březina <pbrezina@redhat.com> Reviewed-by: Tomáš Halman <thalman@redhat.com>

PASSKEY_CHILD: use new helper to setup a process

903c346

This also makes 'passkey_child' to use '--chain-id' if supplied. Reviewed-by: Pavel Březina <pbrezina@redhat.com> Reviewed-by: Tomáš Halman <thalman@redhat.com>

SELINUX_CHILD: use new helper to setup a process

f9acabd

Reviewed-by: Pavel Březina <pbrezina@redhat.com> Reviewed-by: Tomáš Halman <thalman@redhat.com>

SELINUX_CHILD: fix includes

89780cb

Reviewed-by: Pavel Březina <pbrezina@redhat.com> Reviewed-by: Tomáš Halman <thalman@redhat.com>

DUMMY_CHILD TEST: use new helper to setup a process

d850bf7

Reviewed-by: Pavel Březina <pbrezina@redhat.com> Reviewed-by: Tomáš Halman <thalman@redhat.com>

DEBUG: use 'debug_prg_name' if 'debug_log_file' isn't set

89c3556

This avoids using 'sssd' potentially by multiple binaries. Reviewed-by: Pavel Březina <pbrezina@redhat.com> Reviewed-by: Tomáš Halman <thalman@redhat.com>

PROXY_CHILD: use new helper to setup a process

1b5e7ae

Reviewed-by: Pavel Březina <pbrezina@redhat.com> Reviewed-by: Tomáš Halman <thalman@redhat.com>

sssd-bot force-pushed the child-helpers-2 branch from ebdc990 to 1b5e7ae Compare November 21, 2025 13:02

alexey-tikhonov merged commit 6aa7c9a into SSSD:master Nov 21, 2025
17 of 18 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unifying boilerplate code for handling child processes - part 2#8174

Unifying boilerplate code for handling child processes - part 2#8174
alexey-tikhonov merged 17 commits intoSSSD:masterfrom
alexey-tikhonov:child-helpers-2

alexey-tikhonov commented Nov 7, 2025

Uh oh!

alexey-tikhonov commented Nov 13, 2025

Uh oh!

alexey-tikhonov commented Nov 14, 2025

Uh oh!

alexey-tikhonov commented Nov 14, 2025

Uh oh!

alexey-tikhonov commented Nov 18, 2025

Uh oh!

thalman left a comment

Uh oh!

thalman Nov 19, 2025

Uh oh!

alexey-tikhonov Nov 19, 2025

Uh oh!

thalman commented Nov 19, 2025

Uh oh!

thalman left a comment

Uh oh!

pbrezina left a comment

Uh oh!

sssd-bot commented Nov 21, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

alexey-tikhonov commented Nov 7, 2025

Uh oh!

alexey-tikhonov commented Nov 13, 2025

Uh oh!

alexey-tikhonov commented Nov 14, 2025

Uh oh!

alexey-tikhonov commented Nov 14, 2025

Uh oh!

alexey-tikhonov commented Nov 18, 2025

Uh oh!

thalman left a comment

Choose a reason for hiding this comment

Uh oh!

thalman Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

alexey-tikhonov Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

thalman commented Nov 19, 2025

Uh oh!

thalman left a comment

Choose a reason for hiding this comment

Uh oh!

pbrezina left a comment

Choose a reason for hiding this comment

Uh oh!

sssd-bot commented Nov 21, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants