Skip to content

Do not report an error if control_group is already set#15

Merged
rsmarples merged 1 commit intoNetworkConfiguration:masterfrom
mpu:controlgroup
May 9, 2020
Merged

Do not report an error if control_group is already set#15
rsmarples merged 1 commit intoNetworkConfiguration:masterfrom
mpu:controlgroup

Conversation

@mpu
Copy link
Contributor

@mpu mpu commented May 5, 2020

After an update I noticed that dhcpcd reported an error "controlgroup: wheel: not found". Looking into the code it seems that this is due to the new privsep code not moving /etc/group in the chroot. This patch is a quick fix for the error that simply checks if control_group was already parsed successfully.

Here is what the error looked like on my machine:

[root@light ~]# dhcpcd wlp3s0
dhcpcd-9.0.2 starting
controlgroup: wheel: not found
wlp3s0: connected to Access Point `Maison'
...

src/if-options.c Outdated
}
if (grp == NULL) {
logerrx("controlgroup: %s: not found", arg);
if (!ctx->control_group) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you test against != NULL please? It's more explicit and reads better in my eyes.
Also, the bracnes are not needed as the logerrx is just one line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It actually is of type gid_t, so I don't think it'd make sense to compare it to NULL. In control.c you also test it without giving an explicit constant to compare.

src/if-options.c Outdated
grp = getgrnam(arg);
if (grp == NULL) {
logerrx("controlgroup: %s: not found", arg);
if (!ctx->control_group) {
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I now dropped the braces.

@mpu mpu force-pushed the controlgroup branch from bd46bc7 to 7055aab Compare May 6, 2020 21:24
@rsmarples rsmarples merged commit 5cbec32 into NetworkConfiguration:master May 9, 2020
rsmarples added a commit that referenced this pull request Jan 25, 2021
Do not report an error if control_group is already set
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants