From 626839977ef82e00bda86fb04efc102ab485533b Mon Sep 17 00:00:00 2001 From: septs Date: Tue, 11 Mar 2025 10:39:46 +0800 Subject: [PATCH 1/2] refactor: simplify error handling and state updates in lpac-download.c --- .../src/main/jni/lpac-jni/lpac-download.c | 81 +++++++------------ 1 file changed, 28 insertions(+), 53 deletions(-) diff --git a/libs/lpac-jni/src/main/jni/lpac-jni/lpac-download.c b/libs/lpac-jni/src/main/jni/lpac-jni/lpac-download.c index 0562995..7d84253 100644 --- a/libs/lpac-jni/src/main/jni/lpac-jni/lpac-download.c +++ b/libs/lpac-jni/src/main/jni/lpac-jni/lpac-download.c @@ -52,6 +52,8 @@ Java_net_typeblog_lpac_1jni_LpacJni_downloadProfile( jmethodID set_cancelled = (*env)->GetMethodID(env, callback_class, "setCancelled", "(Z)V"); #define IS_CANCELLED (*env)->CallBooleanMethod(env, callback, is_cancelled) +#define CHECK_INVOKE_FAILED(COND) if (COND) { ret = -ES10B_ERROR_REASON_UNDEFINED; goto out; } +#define EMIT_STATE_UPDATE(STATE) (*env)->CallVoidMethod(env, callback, on_state_update, download_state_##STATE) if (confirmation_code != NULL) _confirmation_code = (*env)->GetStringUTFChars(env, confirmation_code, NULL); @@ -63,51 +65,27 @@ Java_net_typeblog_lpac_1jni_LpacJni_downloadProfile( ctx->http.server_address = _smdp; - if (IS_CANCELLED) { - ret = -ES10B_ERROR_REASON_UNDEFINED; - goto out; - } - (*env)->CallVoidMethod(env, callback, on_state_update, download_state_preparing); + CHECK_INVOKE_FAILED(IS_CANCELLED) + EMIT_STATE_UPDATE(preparing); ret = es10b_get_euicc_challenge_and_info(ctx); syslog(LOG_INFO, "es10b_get_euicc_challenge_and_info %d", ret); - if (ret < 0) { - ret = -ES10B_ERROR_REASON_UNDEFINED; - goto out; - } + CHECK_INVOKE_FAILED(ret < 0) - if (IS_CANCELLED) { - ret = -ES10B_ERROR_REASON_UNDEFINED; - goto out; - } - (*env)->CallVoidMethod(env, callback, on_state_update, download_state_connecting); + CHECK_INVOKE_FAILED(IS_CANCELLED) + EMIT_STATE_UPDATE(connecting); ret = es9p_initiate_authentication(ctx); syslog(LOG_INFO, "es9p_initiate_authentication %d", ret); - if (ret < 0) { - ret = -ES10B_ERROR_REASON_UNDEFINED; - goto out; - } + CHECK_INVOKE_FAILED(ret < 0) - if (IS_CANCELLED) { - ret = -ES10B_ERROR_REASON_UNDEFINED; - goto out; - } - (*env)->CallVoidMethod(env, callback, on_state_update, download_state_authenticating); + CHECK_INVOKE_FAILED(IS_CANCELLED) + EMIT_STATE_UPDATE(authenticating); ret = es10b_authenticate_server(ctx, _matching_id, _imei); syslog(LOG_INFO, "es10b_authenticate_server %d", ret); - if (ret < 0) { - ret = -ES10B_ERROR_REASON_UNDEFINED; - goto out; - } + CHECK_INVOKE_FAILED(ret < 0) - if (IS_CANCELLED) { - ret = -ES10B_ERROR_REASON_UNDEFINED; - goto out; - } + CHECK_INVOKE_FAILED(IS_CANCELLED) ret = es9p_authenticate_client(ctx); - if (ret < 0) { - ret = -ES10B_ERROR_REASON_UNDEFINED; - goto out; - } + CHECK_INVOKE_FAILED(ret < 0) const char *b64_profileMetadata = ctx->http._internal.prepare_download_param->b64_profileMetadata; if (b64_profileMetadata != NULL) { @@ -118,33 +96,25 @@ Java_net_typeblog_lpac_1jni_LpacJni_downloadProfile( } (*env)->CallVoidMethod(env, callback, on_profile_metadata, build_profile_metadata(env, profile_metadata)); + if ((*env)->ExceptionCheck(env) == JNI_TRUE) { + ret = -ES10B_ERROR_REASON_UNDEFINED; + goto out; + } } - if (IS_CANCELLED) { - ret = -ES10B_ERROR_REASON_UNDEFINED; - goto out; - } - (*env)->CallVoidMethod(env, callback, on_state_update, download_state_downloading); + CHECK_INVOKE_FAILED(IS_CANCELLED) + EMIT_STATE_UPDATE(downloading); ret = es10b_prepare_download(ctx, _confirmation_code); syslog(LOG_INFO, "es10b_prepare_download %d", ret); - if (ret < 0) { - ret = -ES10B_ERROR_REASON_UNDEFINED; - goto out; - } + CHECK_INVOKE_FAILED(ret < 0) - if (IS_CANCELLED) { - ret = -ES10B_ERROR_REASON_UNDEFINED; - goto out; - } + CHECK_INVOKE_FAILED(IS_CANCELLED) ret = es9p_get_bound_profile_package(ctx); if (ret < 0) goto out; - if (IS_CANCELLED) { - ret = -ES10B_ERROR_REASON_UNDEFINED; - goto out; - } - (*env)->CallVoidMethod(env, callback, on_state_update, download_state_finalizing); + CHECK_INVOKE_FAILED(IS_CANCELLED) + EMIT_STATE_UPDATE(finalizing); ret = es10b_load_bound_profile_package(ctx, &es10b_load_bound_profile_package_result); syslog(LOG_INFO, "es10b_load_bound_profile_package %d, reason %d", ret, es10b_load_bound_profile_package_result.errorReason); @@ -156,11 +126,16 @@ Java_net_typeblog_lpac_1jni_LpacJni_downloadProfile( euicc_http_cleanup(ctx); out: + // isCancelled() == false, but ret is an error, the set to cancelled if (IS_CANCELLED == 0 && ret == -ES10B_ERROR_REASON_UNDEFINED) { (*env)->CallVoidMethod(env, callback, set_cancelled, JNI_TRUE); } es8p_metadata_free(&profile_metadata); + #undef IS_CANCELLED +#undef CHECK_INVOKE_FAILED +#undef EMIT_STATE_UPDATE + // We expect Java side to call cancelSessions after any error -- thus, `euicc_http_cleanup` is done there // This is so that Java side can access the last HTTP and/or APDU errors when we return. if (_confirmation_code != NULL) From a48e44436aa51808ea86be5830e716d8bc5440f7 Mon Sep 17 00:00:00 2001 From: septs Date: Tue, 11 Mar 2025 10:47:33 +0800 Subject: [PATCH 2/2] refactor: organize code into regions for better readability in lpac-download.c --- .../src/main/jni/lpac-jni/lpac-download.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/libs/lpac-jni/src/main/jni/lpac-jni/lpac-download.c b/libs/lpac-jni/src/main/jni/lpac-jni/lpac-download.c index 7d84253..79608a3 100644 --- a/libs/lpac-jni/src/main/jni/lpac-jni/lpac-download.c +++ b/libs/lpac-jni/src/main/jni/lpac-jni/lpac-download.c @@ -65,18 +65,23 @@ Java_net_typeblog_lpac_1jni_LpacJni_downloadProfile( ctx->http.server_address = _smdp; + // region preparing CHECK_INVOKE_FAILED(IS_CANCELLED) EMIT_STATE_UPDATE(preparing); ret = es10b_get_euicc_challenge_and_info(ctx); syslog(LOG_INFO, "es10b_get_euicc_challenge_and_info %d", ret); CHECK_INVOKE_FAILED(ret < 0) + // endregion + // region connecting CHECK_INVOKE_FAILED(IS_CANCELLED) EMIT_STATE_UPDATE(connecting); ret = es9p_initiate_authentication(ctx); syslog(LOG_INFO, "es9p_initiate_authentication %d", ret); CHECK_INVOKE_FAILED(ret < 0) + // endregion + // region authenticating CHECK_INVOKE_FAILED(IS_CANCELLED) EMIT_STATE_UPDATE(authenticating); ret = es10b_authenticate_server(ctx, _matching_id, _imei); @@ -85,8 +90,11 @@ Java_net_typeblog_lpac_1jni_LpacJni_downloadProfile( CHECK_INVOKE_FAILED(IS_CANCELLED) ret = es9p_authenticate_client(ctx); + syslog(LOG_INFO, "es9p_authenticate_client %d", ret); CHECK_INVOKE_FAILED(ret < 0) + // endregion + // region emit profile metadata const char *b64_profileMetadata = ctx->http._internal.prepare_download_param->b64_profileMetadata; if (b64_profileMetadata != NULL) { ret = es8p_metadata_parse(&profile_metadata, b64_profileMetadata); @@ -101,7 +109,9 @@ Java_net_typeblog_lpac_1jni_LpacJni_downloadProfile( goto out; } } + // endregion + // region downloading CHECK_INVOKE_FAILED(IS_CANCELLED) EMIT_STATE_UPDATE(downloading); ret = es10b_prepare_download(ctx, _confirmation_code); @@ -110,9 +120,11 @@ Java_net_typeblog_lpac_1jni_LpacJni_downloadProfile( CHECK_INVOKE_FAILED(IS_CANCELLED) ret = es9p_get_bound_profile_package(ctx); - if (ret < 0) - goto out; + syslog(LOG_INFO, "es9p_get_bound_profile_package %d", ret); + if (ret < 0) goto out; + // endregion + // region finalizing CHECK_INVOKE_FAILED(IS_CANCELLED) EMIT_STATE_UPDATE(finalizing); ret = es10b_load_bound_profile_package(ctx, &es10b_load_bound_profile_package_result); @@ -122,6 +134,7 @@ Java_net_typeblog_lpac_1jni_LpacJni_downloadProfile( ret = -(int) es10b_load_bound_profile_package_result.errorReason; goto out; } + // endregion euicc_http_cleanup(ctx);