Skip to content

system-info: Avoid duplicating rDNS while creating os component #802

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Rirusha wants to merge 1 commit into
base: main
Choose a base branch
from
Open

system-info: Avoid duplicating rDNS while creating os component #802

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions src/as-system-info.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,8 @@ const gchar *
as_system_info_get_os_cid (AsSystemInfo *sysinfo)
{
AsSystemInfoPrivate *priv = GET_PRIVATE (sysinfo);
g_autofree gchar *homepage_cid = NULL;
g_auto(GStrv) os_id_parts = NULL;

as_system_info_load_os_release (sysinfo, NULL);
if (priv->os_cid != NULL)
Expand All @@ -333,6 +335,25 @@ as_system_info_get_os_cid (AsSystemInfo *sysinfo)
return priv->os_cid;
}

homepage_cid = as_utils_dns_to_rdns (priv->os_homepage, NULL);

@ximion ximion Oct 26, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This looks quite redundant - the logic should be "if the ID already looks like rDNS, we simply copy it over as cid and stop guessing from the homepage". So, something like this would be what I would have in mind here:

/* the OS ID may already be an rDNS ID (e.g. org.freedesktop.platform), don't prefix it again */
          if (priv->os_id != NULL) {
                  g_auto(GStrv) id_parts = g_strsplit (priv->os_id, ".", 3);
                  if (g_strv_length (id_parts) == 3 && as_utils_is_tld (id_parts[0])) {
                          priv->os_cid = g_strdup (priv->os_id);
                          return priv->os_cid;
                  }
          }

That way, you can drop the other code and no redundant calls to as_utils_dns_to_rdns happen, that should make this code really small.

if (priv->os_id != NULL) {
if (homepage_cid != NULL) {
gsize homepage_cid_len = strlen (homepage_cid);

if (g_str_has_prefix (priv->os_id, homepage_cid) &&
priv->os_id[homepage_cid_len] == '.') {
priv->os_cid = g_strdup (priv->os_id);
return priv->os_cid;
}
}

os_id_parts = g_strsplit (priv->os_id, ".", -1);
if (g_strv_length (os_id_parts) >= 3 && as_utils_is_tld (os_id_parts[0])) {
priv->os_cid = g_strdup (priv->os_id);
return priv->os_cid;
}
}

priv->os_cid = as_utils_dns_to_rdns (priv->os_homepage, priv->os_id);
if (priv->os_cid == NULL)
return priv->os_id;
Expand Down
3 changes: 3 additions & 0 deletions tests/samples/os-release-rdns-home-match
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
NAME="Freedesktop SDK"
ID=org.freedesktop.platform
HOME_URL="https://freedesktop.org"
3 changes: 3 additions & 0 deletions tests/samples/os-release-rdns-home-match-unknown-tld
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
NAME="Freedesktop Test SDK"
ID=invalid.freedesktop.platform
HOME_URL="https://freedesktop.invalid"
3 changes: 3 additions & 0 deletions tests/samples/os-release-rdns-home-mismatch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
NAME="Example OS"
ID=org.example.platform
HOME_URL="https://project.example.com"
3 changes: 3 additions & 0 deletions tests/samples/os-release-rdns-home-prefix-collision
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
NAME="Freedesktop Prefix Collision Test SDK"
ID=invalid.freedesktopx.platform
HOME_URL="https://freedesktop.invalid"
40 changes: 40 additions & 0 deletions tests/test-basics.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1216,7 +1216,15 @@ test_system_info (void)
{
g_autoptr(AsSystemInfo) sysinfo_tmpl = NULL;
g_autoptr(AsSystemInfo) sysinfo = as_system_info_new ();
g_autoptr(AsSystemInfo) sysinfo_rdns_home_match = as_system_info_new ();
g_autoptr(AsSystemInfo) sysinfo_rdns_home_match_unknown_tld = as_system_info_new ();
g_autoptr(AsSystemInfo) sysinfo_rdns_home_prefix_collision = as_system_info_new ();
g_autoptr(AsSystemInfo) sysinfo_rdns_home_mismatch = as_system_info_new ();
g_autofree gchar *osrelease_fname = NULL;
g_autofree gchar *osrelease_rdns_home_match_fname = NULL;
g_autofree gchar *osrelease_rdns_home_match_unknown_tld_fname = NULL;
g_autofree gchar *osrelease_rdns_home_prefix_collision_fname = NULL;
g_autofree gchar *osrelease_rdns_home_mismatch_fname = NULL;
g_autofree gchar *dev_name = NULL;
g_autoptr(GError) error = NULL;

Expand All @@ -1231,6 +1239,38 @@ test_system_info (void)
g_assert_cmpstr (as_system_info_get_os_id (sysinfo), ==, "debian");
g_assert_cmpstr (as_system_info_get_os_cid (sysinfo), ==, "org.debian.debian");

osrelease_rdns_home_match_fname =
g_build_filename (datadir, "os-release-rdns-home-match", NULL);
as_system_info_load_os_release (sysinfo_rdns_home_match,
osrelease_rdns_home_match_fname);
g_assert_cmpstr (as_system_info_get_os_cid (sysinfo_rdns_home_match),
==,
"org.freedesktop.platform");

osrelease_rdns_home_match_unknown_tld_fname =
g_build_filename (datadir, "os-release-rdns-home-match-unknown-tld", NULL);
as_system_info_load_os_release (sysinfo_rdns_home_match_unknown_tld,
osrelease_rdns_home_match_unknown_tld_fname);
g_assert_cmpstr (as_system_info_get_os_cid (sysinfo_rdns_home_match_unknown_tld),
==,
"invalid.freedesktop.platform");

osrelease_rdns_home_prefix_collision_fname =
g_build_filename (datadir, "os-release-rdns-home-prefix-collision", NULL);
as_system_info_load_os_release (sysinfo_rdns_home_prefix_collision,
osrelease_rdns_home_prefix_collision_fname);
g_assert_cmpstr (as_system_info_get_os_cid (sysinfo_rdns_home_prefix_collision),
==,
"invalid.freedesktop.invalid.freedesktopx.platform");

osrelease_rdns_home_mismatch_fname =
g_build_filename (datadir, "os-release-rdns-home-mismatch", NULL);
as_system_info_load_os_release (sysinfo_rdns_home_mismatch,
osrelease_rdns_home_mismatch_fname);
g_assert_cmpstr (as_system_info_get_os_cid (sysinfo_rdns_home_mismatch),
==,
"org.example.platform");

#ifndef G_OS_WIN32
g_assert_nonnull (as_system_info_get_kernel_name (sysinfo));
g_assert_nonnull (as_system_info_get_kernel_version (sysinfo));
Expand Down
Loading