Skip to content

systemd: allow AF_NETLINK when upnp is enabled#2525

Open
abouvier wants to merge 1 commit into
MusicPlayerDaemon:masterfrom
abouvier:patch-1
Open

systemd: allow AF_NETLINK when upnp is enabled#2525
abouvier wants to merge 1 commit into
MusicPlayerDaemon:masterfrom
abouvier:patch-1

Conversation

@abouvier

Copy link
Copy Markdown

AF_NETLINK is also needed for the upnp database plugin: https://bbs.archlinux.org/viewtopic.php?id=310134

@MaxKellermann

Copy link
Copy Markdown
Member

Bad commit message

@abouvier

Copy link
Copy Markdown
Author

Where is the guideline for good commit message? :p

@MaxKellermann

Copy link
Copy Markdown
Member

Rule of thumb: if the PR contains text that is missing in your commit message, then your commit message must be obviously bad.

Second rule of thumb: if your commit message describes only what is obvious enough by looking at the diff, but doesn't even try to explain why, it's bad.

@abouvier abouvier changed the title fix upnp database plugin systemd: allow AF_NETLINK when upnp is enabled Jun 26, 2026
@MaxKellermann

Copy link
Copy Markdown
Member

Your commit message says "when upnp is enabled" but that's not what your actual code does - it checks whether the feature is not disabled. That's different from the existing smbclient check, and that has a bad smell. That needs to be explained.

@abouvier

Copy link
Copy Markdown
Author

Because the smbclient option is a boolean, but the upnp option is a combo with multiple ways to be enabled, so checking if it's not disabled is simpler.

@MaxKellermann

Copy link
Copy Markdown
Member

Because the smbclient option is a boolean,

This is wrong. It's a feature.

@MaxKellermann

Copy link
Copy Markdown
Member

Explanation still missing.

@MaxKellermann MaxKellermann added the waiting Waiting for more information from reporter label Jun 30, 2026
mpd.service will fail to start when mpd is configured with upnp database/neighbor plugin
and AF_NETLINK is not allowed in RestrictAddressFamilies=
@abouvier

abouvier commented Jul 2, 2026

Copy link
Copy Markdown
Author

get_option('upnp').enabled() is not possible because this option is not a feature but a combo, so the value returned by get_option will be a string.

Besides, multiple values of this option can end up enabling the upnp plugin, with the logic to determine that in src/lib/upnp/meson.build. So I guess that the best way to verify for sure that upnp is enabled is to check the configuration variable set in this file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting Waiting for more information from reporter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants