From 20230ec62594fc7d8a6579b1efbd796f74148a9b Mon Sep 17 00:00:00 2001 From: jdgleaver Date: Mon, 31 Aug 2020 12:12:09 +0100 Subject: [PATCH] (Android) Play Store builds: Do not resolve symlinks when handling core paths --- core_updater_list.c | 21 +++- menu/cbs/menu_cbs_ok.c | 6 +- play_feature_delivery/play_feature_delivery.c | 96 +++++++++++++++---- playlist.c | 34 +++++-- playlist.h | 5 +- retroarch.c | 13 ++- 6 files changed, 138 insertions(+), 37 deletions(-) diff --git a/core_updater_list.c b/core_updater_list.c index 80c8b32aea..2f78f5ca29 100644 --- a/core_updater_list.c +++ b/core_updater_list.c @@ -273,6 +273,7 @@ bool core_updater_list_get_core( const char *local_core_path, const core_updater_list_entry_t **entry) { + bool resolve_symlinks; size_t num_entries; size_t i; char real_core_path[PATH_MAX_LENGTH]; @@ -289,7 +290,14 @@ bool core_updater_list_get_core( /* Resolve absolute pathname of local_core_path */ strlcpy(real_core_path, local_core_path, sizeof(real_core_path)); - path_resolve_realpath(real_core_path, sizeof(real_core_path), true); + /* Can't resolve symlinks when dealing with cores + * installed via play feature delivery, because the + * source files have non-standard file names (which + * will not be recognised by regular core handling + * routines) */ + resolve_symlinks = (core_list->type != CORE_UPDATER_LIST_TYPE_PFD); + path_resolve_realpath(real_core_path, sizeof(real_core_path), + resolve_symlinks); if (string_is_empty(real_core_path)) return false; @@ -391,10 +399,16 @@ static bool core_updater_list_set_paths( { char *last_underscore = NULL; char *tmp_url = NULL; + bool is_archive = true; + /* Can't resolve symlinks when dealing with cores + * installed via play feature delivery, because the + * source files have non-standard file names (which + * will not be recognised by regular core handling + * routines) */ + bool resolve_symlinks = (list_type != CORE_UPDATER_LIST_TYPE_PFD); char remote_core_path[PATH_MAX_LENGTH]; char local_core_path[PATH_MAX_LENGTH]; char local_info_path[PATH_MAX_LENGTH]; - bool is_archive; remote_core_path[0] = '\0'; local_core_path[0] = '\0'; @@ -460,7 +474,8 @@ static bool core_updater_list_set_paths( if (is_archive) path_remove_extension(local_core_path); - path_resolve_realpath(local_core_path, sizeof(local_core_path), true); + path_resolve_realpath(local_core_path, sizeof(local_core_path), + resolve_symlinks); if (entry->local_core_path) { diff --git a/menu/cbs/menu_cbs_ok.c b/menu/cbs/menu_cbs_ok.c index e2f489bb06..08f0ef875b 100644 --- a/menu/cbs/menu_cbs_ok.c +++ b/menu/cbs/menu_cbs_ok.c @@ -2168,7 +2168,7 @@ static int action_ok_playlist_entry_collection(const char *path, if (!string_is_empty(entry->path)) { strlcpy(content_path, entry->path, sizeof(content_path)); - playlist_resolve_path(PLAYLIST_LOAD, content_path, sizeof(content_path)); + playlist_resolve_path(PLAYLIST_LOAD, false, content_path, sizeof(content_path)); } /* Cache entry label */ @@ -2239,7 +2239,7 @@ static int action_ok_playlist_entry_collection(const char *path, /* Core path is invalid - just copy what we have * and hope for the best... */ strlcpy(core_path, entry->core_path, sizeof(core_path)); - playlist_resolve_path(PLAYLIST_LOAD, core_path, sizeof(core_path)); + playlist_resolve_path(PLAYLIST_LOAD, true, core_path, sizeof(core_path)); } } } @@ -3284,7 +3284,7 @@ static int action_ok_core_deferred_set(const char *new_core_path, true); strlcpy(resolved_core_path, new_core_path, sizeof(resolved_core_path)); - playlist_resolve_path(PLAYLIST_SAVE, resolved_core_path, sizeof(resolved_core_path)); + playlist_resolve_path(PLAYLIST_SAVE, true, resolved_core_path, sizeof(resolved_core_path)); /* the update function reads our entry * as const, so these casts are safe */ diff --git a/play_feature_delivery/play_feature_delivery.c b/play_feature_delivery/play_feature_delivery.c index 357c2dc37b..ac25b844aa 100644 --- a/play_feature_delivery/play_feature_delivery.c +++ b/play_feature_delivery/play_feature_delivery.c @@ -44,22 +44,28 @@ typedef struct { #ifdef HAVE_THREADS - slock_t *lock; + slock_t *enabled_lock; + slock_t *status_lock; #endif unsigned download_progress; enum play_feature_delivery_install_status last_status; char last_core_name[256]; + bool enabled; + bool enabled_set; bool active; } play_feature_delivery_state_t; static play_feature_delivery_state_t play_feature_delivery_state = { #ifdef HAVE_THREADS - NULL, /* lock */ + NULL, /* enabled_lock */ + NULL, /* status_lock */ #endif 0, /* download_progress */ PLAY_FEATURE_DELIVERY_IDLE, /* last_status */ {'\0'}, /* last_core_name */ + false, /* enabled */ + false, /* enabled_set */ false, /* active */ }; @@ -85,7 +91,7 @@ JNIEXPORT void JNICALL Java_com_retroarch_browser_retroactivity_RetroActivityCom /* Lock mutex */ #ifdef HAVE_THREADS - slock_lock(state->lock); + slock_lock(state->status_lock); #endif /* Only update status if an install is active */ @@ -114,7 +120,7 @@ JNIEXPORT void JNICALL Java_com_retroarch_browser_retroactivity_RetroActivityCom /* Unlock mutex */ #ifdef HAVE_THREADS - slock_unlock(state->lock); + slock_unlock(state->status_lock); #endif } @@ -130,7 +136,7 @@ JNIEXPORT void JNICALL Java_com_retroarch_browser_retroactivity_RetroActivityCom /* Lock mutex */ #ifdef HAVE_THREADS - slock_lock(state->lock); + slock_lock(state->status_lock); #endif /* Only update status if an install is active */ @@ -184,7 +190,7 @@ JNIEXPORT void JNICALL Java_com_retroarch_browser_retroactivity_RetroActivityCom /* Unlock mutex */ #ifdef HAVE_THREADS - slock_unlock(state->lock); + slock_unlock(state->status_lock); #endif } @@ -198,12 +204,23 @@ void play_feature_delivery_init(void) play_feature_delivery_state_t* state = play_feature_delivery_get_state(); play_feature_delivery_deinit(); -#ifdef HAVE_THREADS - if (!state->lock) - state->lock = slock_new(); - retro_assert(state->lock); +#ifdef HAVE_THREADS + if (!state->enabled_lock) + state->enabled_lock = slock_new(); + + retro_assert(state->enabled_lock); + + if (!state->status_lock) + state->status_lock = slock_new(); + + retro_assert(state->status_lock); #endif + + /* Note: Would like to cache whether this + * is a Play Store build here, but + * play_feature_delivery_init() is called + * too early in the startup sequence... */ } /* Must be called upon program termination */ @@ -212,10 +229,16 @@ void play_feature_delivery_deinit(void) play_feature_delivery_state_t* state = play_feature_delivery_get_state(); #ifdef HAVE_THREADS - if (state->lock) + if (state->enabled_lock) { - slock_free(state->lock); - state->lock = NULL; + slock_free(state->enabled_lock); + state->enabled_lock = NULL; + } + + if (state->status_lock) + { + slock_free(state->status_lock); + state->status_lock = NULL; } #endif } @@ -251,8 +274,10 @@ static bool play_feature_delivery_get_core_name( } /* Returns true if current build utilises - * play feature delivery */ -bool play_feature_delivery_enabled(void) + * play feature delivery + * > Relies on a Java function call, and is + * therefore slow */ +static bool play_feature_delivery_enabled_internal(void) { JNIEnv *env = jni_thread_getenv(); struct android_app *app = (struct android_app*)g_android; @@ -269,6 +294,39 @@ bool play_feature_delivery_enabled(void) return enabled; } +/* Returns true if current build utilises + * play feature delivery */ +bool play_feature_delivery_enabled(void) +{ + play_feature_delivery_state_t* state = play_feature_delivery_get_state(); + bool enabled; + + /* Lock mutex */ +#ifdef HAVE_THREADS + slock_lock(state->enabled_lock); +#endif + + /* Calling Java functions is slow. We need to + * check Play Store build status frequently, + * often in loops, so rely on a cached global + * status flag instead dealing with Java + * interfaces */ + if (!state->enabled_set) + { + state->enabled = play_feature_delivery_enabled_internal(); + state->enabled_set = true; + } + + enabled = state->enabled; + + /* Unlock mutex */ +#ifdef HAVE_THREADS + slock_unlock(state->enabled_lock); +#endif + + return enabled; +} + /* Returns a list of cores currently available * via play feature delivery. * Returns a new string_list on success, or @@ -408,7 +466,7 @@ bool play_feature_delivery_download_status( /* Lock mutex */ #ifdef HAVE_THREADS - slock_lock(state->lock); + slock_lock(state->status_lock); #endif /* Copy status parameters */ @@ -422,7 +480,7 @@ bool play_feature_delivery_download_status( /* Unlock mutex */ #ifdef HAVE_THREADS - slock_unlock(state->lock); + slock_unlock(state->status_lock); #endif return active; @@ -459,7 +517,7 @@ bool play_feature_delivery_download(const char *core_file) /* Lock mutex */ #ifdef HAVE_THREADS - slock_lock(state->lock); + slock_lock(state->status_lock); #endif /* We only support one download at a time */ @@ -487,7 +545,7 @@ bool play_feature_delivery_download(const char *core_file) /* Unlock mutex */ #ifdef HAVE_THREADS - slock_unlock(state->lock); + slock_unlock(state->status_lock); #endif return success; diff --git a/playlist.c b/playlist.c index 60ba20402f..abfa09cad1 100644 --- a/playlist.c +++ b/playlist.c @@ -334,7 +334,8 @@ static bool playlist_core_path_equal(const char *real_core_path, const char *ent strlcpy(entry_real_core_path, entry_core_path, sizeof(entry_real_core_path)); if (!string_is_equal(entry_real_core_path, FILE_PATH_DETECT) && !string_is_equal(entry_real_core_path, FILE_PATH_BUILTIN)) - path_resolve_realpath(entry_real_core_path, sizeof(entry_real_core_path), true); + playlist_resolve_path(PLAYLIST_SAVE, true, entry_real_core_path, + sizeof(entry_real_core_path)); if (string_is_empty(entry_real_core_path)) return false; @@ -764,14 +765,15 @@ bool playlist_push_runtime(playlist_t *playlist, if (!string_is_empty(entry->path)) { strlcpy(real_path, entry->path, sizeof(real_path)); - path_resolve_realpath(real_path, sizeof(real_path), true); + playlist_resolve_path(PLAYLIST_SAVE, false, real_path, sizeof(real_path)); } /* Get 'real' core path */ strlcpy(real_core_path, entry->core_path, sizeof(real_core_path)); if (!string_is_equal(real_core_path, FILE_PATH_DETECT) && !string_is_equal(real_core_path, FILE_PATH_BUILTIN)) - path_resolve_realpath(real_core_path, sizeof(real_core_path), true); + playlist_resolve_path(PLAYLIST_SAVE, true, real_core_path, + sizeof(real_core_path)); if (string_is_empty(real_core_path)) { @@ -869,6 +871,7 @@ success: /** * playlist_resolve_path: * @mode : PLAYLIST_LOAD or PLAYLIST_SAVE + * @is_core : Set true if path to be resolved is a core file * @path : The path to be modified * * Resolves the path of an item, such as the content path or path to the core, to a format @@ -879,7 +882,7 @@ success: * install (iOS) **/ void playlist_resolve_path(enum playlist_file_mode mode, - char *path, size_t len) + bool is_core, char *path, size_t len) { #ifdef HAVE_COCOATOUCH char tmp[PATH_MAX_LENGTH]; @@ -903,10 +906,22 @@ void playlist_resolve_path(enum playlist_file_mode mode, fill_pathname_abbreviate_special(path, tmp2, len); } #else + bool resolve_symlinks = true; + if (mode == PLAYLIST_LOAD) return; - path_resolve_realpath(path, len, true); +#if defined(ANDROID) + /* Can't resolve symlinks when dealing with cores + * installed via play feature delivery, because the + * source files have non-standard file names (which + * will not be recognised by regular core handling + * routines) */ + if (is_core) + resolve_symlinks = !play_feature_delivery_enabled(); +#endif + + path_resolve_realpath(path, len, resolve_symlinks); #endif } @@ -941,14 +956,14 @@ bool playlist_push(playlist_t *playlist, if (!string_is_empty(entry->path)) { strlcpy(real_path, entry->path, sizeof(real_path)); - playlist_resolve_path(PLAYLIST_SAVE, real_path, sizeof(real_path)); + playlist_resolve_path(PLAYLIST_SAVE, false, real_path, sizeof(real_path)); } /* Get 'real' core path */ strlcpy(real_core_path, entry->core_path, sizeof(real_core_path)); if (!string_is_equal(real_core_path, FILE_PATH_DETECT) && !string_is_equal(real_core_path, FILE_PATH_BUILTIN)) - playlist_resolve_path(PLAYLIST_SAVE, real_core_path, + playlist_resolve_path(PLAYLIST_SAVE, true, real_core_path, sizeof(real_core_path)); if (string_is_empty(real_core_path)) @@ -2976,7 +2991,8 @@ bool playlist_entries_are_equal( strlcpy(real_core_path_a, entry_a->core_path, sizeof(real_core_path_a)); if (!string_is_equal(real_core_path_a, FILE_PATH_DETECT) && !string_is_equal(real_core_path_a, FILE_PATH_BUILTIN)) - path_resolve_realpath(real_core_path_a, sizeof(real_core_path_a), true); + playlist_resolve_path(PLAYLIST_SAVE, true, + real_core_path_a, sizeof(real_core_path_a)); } return playlist_core_path_equal(real_core_path_a, entry_b->core_path, config); @@ -3076,7 +3092,7 @@ void playlist_set_default_core_path(playlist_t *playlist, const char *core_path) strlcpy(real_core_path, core_path, sizeof(real_core_path)); if (!string_is_equal(real_core_path, FILE_PATH_DETECT) && !string_is_equal(real_core_path, FILE_PATH_BUILTIN)) - playlist_resolve_path(PLAYLIST_SAVE, + playlist_resolve_path(PLAYLIST_SAVE, true, real_core_path, sizeof(real_core_path)); if (string_is_empty(real_core_path)) diff --git a/playlist.h b/playlist.h index e3f27a91b0..7750455a02 100644 --- a/playlist.h +++ b/playlist.h @@ -223,7 +223,8 @@ void playlist_delete_by_path(playlist_t *playlist, /** * playlist_resolve_path: * @mode : PLAYLIST_LOAD or PLAYLIST_SAVE - * @path : The path to be modified + * @is_core : Set true if path to be resolved is a core file + * @path : The path to be modified * * Resolves the path of an item, such as the content path or path to the core, to a format * appropriate for saving or loading depending on the @mode parameter @@ -233,7 +234,7 @@ void playlist_delete_by_path(playlist_t *playlist, * install (iOS) **/ void playlist_resolve_path(enum playlist_file_mode mode, - char *path, size_t len); + bool is_core, char *path, size_t len); /** * playlist_push: diff --git a/retroarch.c b/retroarch.c index a379561709..9dd3830c15 100644 --- a/retroarch.c +++ b/retroarch.c @@ -20305,6 +20305,17 @@ static bool load_dynamic_core( struct rarch_state *p_rarch, const char *path, char *buf, size_t size) { +#if defined(ANDROID) + /* Can't resolve symlinks when dealing with cores + * installed via play feature delivery, because the + * source files have non-standard file names (which + * will not be recognised by regular core handling + * routines) */ + bool resolve_symlinks = !play_feature_delivery_enabled(); +#else + bool resolve_symlinks = true; +#endif + /* Can't lookup symbols in itself on UWP */ #if !(defined(__WINRT__) || defined(WINAPI_FAMILY) && WINAPI_FAMILY == WINAPI_FAMILY_PHONE_APP) if (dylib_proc(NULL, "retro_init")) @@ -20323,7 +20334,7 @@ static bool load_dynamic_core( /* Need to use absolute path for this setting. It can be * saved to content history, and a relative path would * break in that scenario. */ - path_resolve_realpath(buf, size, true); + path_resolve_realpath(buf, size, resolve_symlinks); if ((p_rarch->lib_handle = dylib_load(path))) return true; return false;