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

Already on GitHub? Sign in to your account

[droid] Check external storage state before using it #1850

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Contributor

mikrohard commented Nov 27, 2012

I have some Android boxes which provide an writeable external storage path (into RAM?) even if there is no external media mounted. As a result XBMC looses all it's data after a reboot.

This commit adds a check for external storage state mounted before trying to get the path. This fixes the problem on my boxes. Tested with and without an SD card.

@Montellese Montellese and 1 other commented on an outdated diff Nov 27, 2012

xbmc/android/activity/android_main.cpp
- // Get the path to the external storage
- // The path would actually be available from state->activity->externalDataPath (apart from a (known) bug in Android 2.3.x)
- // but calling getExternalFilesDir() will automatically create the necessary directories for us.
- // There might NOT be external storage present so check it before trying to setup.
+ // get android external storage state
+ jclass cAndroidEnv = env->FindClass("android/os/Environment");
+ jmethodID midActivityExternalStorageState = env->GetStaticMethodID(cAndroidEnv, "getExternalStorageState", "()Ljava/lang/String;");
+ jstring sExternalStorageState = (jstring)env->CallStaticObjectMethod(cAndroidEnv, midActivityExternalStorageState);
+ const char* strExternalStorageState = env->GetStringUTFChars(sExternalStorageState, NULL);
+ jfieldID fidMediaMounted = env->GetStaticFieldID(cAndroidEnv, "MEDIA_MOUNTED", "Ljava/lang/String;");
+ jstring sMediaMounted = (jstring)env->GetStaticObjectField(cAndroidEnv, fidMediaMounted);
+ const char* strMediaMounted = env->GetStringUTFChars(sMediaMounted, NULL);
+ bool bExternalStorageMounted = strcmp(strExternalStorageState, strMediaMounted) == 0;
+ env->ReleaseStringUTFChars(sExternalStorageState, strExternalStorageState);
+ env->DeleteLocalRef(sExternalStorageState);
+ env->ReleaseStringUTFChars(sExternalStorageState, strMediaMounted);
@Montellese

Montellese Nov 27, 2012

Owner

Should be sMediaMounted instead of sExternalStorageState.

@mikrohard

mikrohard Nov 27, 2012

Contributor

Oh... sure... C/P error. Will fix right away :)

Owner

Montellese commented Nov 27, 2012

I didn't try it but it looks reasonable (apart from the comment I made). It might be better to move the calls to DeleteLocalRef further up right below the last usage of the reference because we only have a limited amount of references available so the sooner we get rid of them the better. But as this happens at the very start where no other JNI stuff is running, it isn't such a big deal.

Contributor

mikrohard commented Nov 27, 2012

It was added in API version 1 and therefore it should always exist: http://developer.android.com/reference/android/os/Environment.html

But I'm no Android expert... if anyone thinks that null checks are necessary I'll add them...

Owner

MartijnKaijser commented Jul 24, 2013

@mikrohard can you rebase on master?

Contributor

mikrohard commented Jul 24, 2013

Done. I hope everything is alright because I'm on vacation (and thus unable to build&test).

@mikrohard mikrohard closed this Oct 3, 2013

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