Skip to content
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.

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

XdpAppInfo cleanups #1619

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
17 changes: 9 additions & 8 deletions src/xdp-app-info-flatpak.c
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ flatpak_is_valid_name (const char *string)
return TRUE;
}

gboolean
static gboolean
xdp_app_info_flatpak_is_valid_sub_app_id (XdpAppInfo *app_info,
const char *sub_app_id)
{
Expand Down Expand Up @@ -208,11 +208,11 @@ xdp_app_info_flatpak_remap_path (XdpAppInfo *app_info,
NULL);

/* For apps we translate /app and /usr to the installed locations.
Also, we need to rewrite to drop the /newroot prefix added by
bubblewrap for other files to work. See
https://github.com/projectatomic/bubblewrap/pull/172
for a bit more information on the /newroot issue.
*/
* Also, we need to rewrite to drop the /newroot prefix added by
* bubblewrap for other files to work. See
* https://github.com/projectatomic/bubblewrap/pull/172
* for a bit more information on the /newroot issue.
*/

if (g_str_has_prefix (path, "/newroot/"))
path = path + strlen ("/newroot");
Expand Down Expand Up @@ -807,7 +807,8 @@ xdp_app_info_flatpak_new (int pid,
/* flatpak has a xdg-dbus-proxy running which means we can't get the pidfd
* of the connected process but we can get the pidfd of the bwrap instance
* instead. This is okay because it has the same namespaces as the calling
* process. */
* process.
*/
bwrap_pidfd = get_bwrap_pidfd (instance, error);
if (bwrap_pidfd == -1)
return NULL;
Expand All @@ -818,7 +819,7 @@ xdp_app_info_flatpak_new (int pid,
xdp_app_info_initialize (XDP_APP_INFO (app_info_flatpak),
FLATPAK_ENGINE_ID, id, instance,
bwrap_pidfd, gappinfo,
TRUE, has_network, TRUE);
TRUE, has_network);
app_info_flatpak->flatpak_info = g_steal_pointer (&metadata);

return XDP_APP_INFO (g_steal_pointer (&app_info_flatpak));
Expand Down
9 changes: 4 additions & 5 deletions src/xdp-app-info-host.c
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,14 @@ xdp_app_info_host_get_usb_queries (XdpAppInfo *app_info)
return app_info_host->usb_queries;
}

gboolean
static gboolean
xdp_app_info_host_is_valid_sub_app_id (XdpAppInfo *app_info,
const char *sub_app_id)
{
return TRUE;
}

gboolean
static gboolean
xdp_app_info_host_validate_autostart (XdpAppInfo *app_info,
GKeyFile *keyfile,
const char * const *autostart_exec,
Expand All @@ -61,7 +61,7 @@ xdp_app_info_host_validate_autostart (XdpAppInfo *app_info,
return TRUE;
}

gboolean
static gboolean
xdp_app_info_host_validate_dynamic_launcher (XdpAppInfo *app_info,
GKeyFile *key_file,
GError **error)
Expand Down Expand Up @@ -198,8 +198,7 @@ xdp_app_info_host_new_full (const char *app_id,
NULL, app_id, NULL,
pidfd, gappinfo,
/* supports_opath */ TRUE,
/* has_network */ TRUE,
/* requires_pid_mapping */ FALSE);
/* has_network */ TRUE);

return app_info_host;
}
Expand Down
3 changes: 1 addition & 2 deletions src/xdp-app-info-private.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,5 +53,4 @@ void xdp_app_info_initialize (XdpAppInfo *app_info,
int pidfd,
GAppInfo *gappinfo,
gboolean supports_opath,
gboolean has_network,
gboolean requires_pid_mapping);
gboolean has_network);
2 changes: 1 addition & 1 deletion src/xdp-app-info-snap.c
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ xdp_app_info_snap_new (int pid,
xdp_app_info_initialize (XDP_APP_INFO (app_info_snap),
"io.snapcraft", snap_id, NULL,
pidfd, gappinfo,
FALSE, has_network, TRUE);
FALSE, has_network);

return XDP_APP_INFO (g_steal_pointer (&app_info_snap));
}
4 changes: 2 additions & 2 deletions src/xdp-app-info-test.c
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ xdp_app_info_test_get_usb_queries (XdpAppInfo *app_info)
return app_info_test->usb_queries;
}

gboolean
static gboolean
xdp_app_info_test_is_valid_sub_app_id (XdpAppInfo *app_info,
const char *sub_app_id)
{
Expand Down Expand Up @@ -137,7 +137,7 @@ xdp_app_info_test_new (const char *app_id,
xdp_app_info_initialize (XDP_APP_INFO (app_info_test),
"", app_id, NULL,
-1, NULL,
TRUE, TRUE, TRUE);
TRUE, TRUE);

app_info_test->usb_queries = parse_usb_queries_string (usb_queries_str);

Expand Down
37 changes: 18 additions & 19 deletions src/xdp-app-info.c
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ typedef struct _XdpAppInfoPrivate
GAppInfo *gappinfo;
gboolean supports_opath;
gboolean has_network;
gboolean requires_pid_mapping;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe I screwed something up when refactoring XdpAppInfo? Might be worth double checking what this was used for...

Copy link
Member Author

Choose a reason for hiding this comment

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

It was never used:


/* pid namespace mapping */
GMutex pidns_lock;
Expand Down Expand Up @@ -118,8 +117,7 @@ xdp_app_info_initialize (XdpAppInfo *app_info,
int pidfd,
GAppInfo *gappinfo,
gboolean supports_opath,
gboolean has_network,
gboolean requires_pid_mapping)
gboolean has_network)
{
XdpAppInfoPrivate *priv = xdp_app_info_get_instance_private (app_info);

Expand All @@ -130,7 +128,6 @@ xdp_app_info_initialize (XdpAppInfo *app_info,
g_set_object (&priv->gappinfo, gappinfo);
priv->supports_opath = supports_opath;
priv->has_network = has_network;
priv->requires_pid_mapping = requires_pid_mapping;
}

gboolean
Expand Down Expand Up @@ -234,7 +231,6 @@ static char *
remap_path (XdpAppInfo *app_info,
const char *path)
{

if (!XDP_APP_INFO_GET_CLASS (app_info)->remap_path)
return g_strdup (path);

Expand Down Expand Up @@ -262,8 +258,9 @@ verify_proc_self_fd (XdpAppInfo *app_info,
path_buffer[symlink_size] = 0;

/* All normal paths start with /, but some weird things
don't, such as socket:[27345] or anon_inode:[eventfd].
We don't support any of these */
* don't, such as socket:[27345] or anon_inode:[eventfd].
* We don't support any of these.
*/
if (path_buffer[0] != '/')
{
g_set_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_FILENAME,
Expand All @@ -272,23 +269,25 @@ verify_proc_self_fd (XdpAppInfo *app_info,
}

/* File descriptors to actually deleted files have " (deleted)"
appended to them. This also happens to some fake fd types
like shmem which are "/<name> (deleted)". All such
files are considered invalid. Unfortunately this also
matches files with filenames that actually end in " (deleted)",
but there is not much to do about this. */
* appended to them. This also happens to some fake fd types
* like shmem which are "/<name> (deleted)". All such
* files are considered invalid. Unfortunately this also
* matches files with filenames that actually end in " (deleted)",
* but there is not much to do about this.
*/
if (g_str_has_suffix (path_buffer, " (deleted)"))
{
const char *mountpoint = xdp_get_documents_mountpoint ();

if (mountpoint != NULL && g_str_has_prefix (path_buffer, mountpoint))
{
/* Unfortunately our workaround for dcache purging triggers
o_path file descriptors on the fuse filesystem being
marked as deleted, so we have to allow these here and
rewrite them. This is safe, becase we will stat the file
and compare to make sure we end up on the right file. */
path_buffer[symlink_size - strlen(" (deleted)")] = 0;
* o_path file descriptors on the fuse filesystem being
* marked as deleted, so we have to allow these here and
* rewrite them. This is safe, becase we will stat the file
* and compare to make sure we end up on the right file.
*/
path_buffer[symlink_size - strlen (" (deleted)")] = 0;
}
else
{
Expand Down Expand Up @@ -876,7 +875,7 @@ xdp_connection_create_host_app_info_sync (GDBusConnection *connection,
if (is_sandboxed)
{
g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
"Can't manually register a flatpak application");
"Can't manually register a Flatpak application");
return NULL;
}

Expand All @@ -886,7 +885,7 @@ xdp_connection_create_host_app_info_sync (GDBusConnection *connection,
if (is_sandboxed)
{
g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
"Can't manually register a snap application");
"Can't manually register a Snap application");
return NULL;
}

Expand Down