Skip to content

Commit

Permalink
Fix bug with sd card partitioning.
Browse files Browse the repository at this point in the history 
Change-Id: I84d1f641c7d493c268dcd26fceba863a042c490d
  • Loading branch information
@koush
koush committed Nov 24, 2012
1 parent 038bd90 commit d3efd1f
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion extendedcommands.c
View file
Expand Up @@ -1261,7 +1261,7 @@ int can_partition(const char* volume) {

int vol_len = strlen(vol->device);
// do not allow partitioning of a device that isn't mmcblkX or mmcblkXp1
if (vol->device[vol_len - 2] == 'p' && vol->device[vol_len - 2] != '1') {
if (vol->device[vol_len - 2] == 'p' && vol->device[vol_len - 1] != '1') {
LOGI("Can't partition unsafe device: %s\n", vol->device);
return 0;
}
Expand Down

6 comments on commit d3efd1f

@PhilZ-cwm6
Copy link

Choose a reason for hiding this comment

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

Not yet, here's a definitive fix:

My fstab:
/emmc vfat /dev/block/mmcblk0p11
/sdcard vfat /dev/block/mmcblk1p1

actual code:

static char* list[] = { "Reboot Recovery",
                        "Wipe Dalvik Cache",
                        "Wipe Battery Stats",
                        "Report Error",
                        "Key Test",
                        "Show log",
                        "Fix Permissions",
                        "Partition sdcard",
                        "Partition External sdcard",
                        "Partition Internal sdcard",
                        NULL
};

if (!can_partition("/sdcard")) {
    list[7] = NULL;
}
if (!can_partition("/external_sd")) {
    list[8] = NULL;
}
if (!can_partition("/emmc")) {
    list[9] = NULL;
}

What happens:
First enter Advanced menu: all is displayed ok: list 7: sdcard, list 8: null, list 9: /emmc

Back to main menu, then return to advanced menu, what happens:
list 7: /sdcard, list 8: NULL ----> since it was memorized as /emmc, we loose the option to format /emmc, there is no list [9]

----> you can format /sdcard only, /emmc is no more displayed to format

*************** Clean fix method **********

static char* list[] = { "Reboot Recovery",
                        "Wipe Dalvik Cache",
                        "Wipe Battery Stats",
                        "Report Error",
                        "Key Test",
                        "Show log",
                        "Fix Permissions",
                        NULL,
                        NULL,
                        NULL
};

char *other_sd = NULL;
if (volume_for_path("/emmc") != NULL) {
    other_sd = "/emmc";
    list[7] = "Partition External sdcard";
    list[8] = "Partition Internal sdcard";
} else if (volume_for_path("/external_sd") != NULL) {
    other_sd = "/external_sd";
    list[7] = "Partition Internal sdcard";
    list[8] = "Partition External sdcard";
}

.
.
.

        case 7:
            if (ensure_path_mounted("/sdcard") != 0) {
                ui_print("Can't mount /sdcard\n");
                break;
            }
            if (can_partition("/sdcard")) {
                partition_sdcard("/sdcard");
            }
            break;
        case 8:
            if (ensure_path_mounted(other_sd) != 0) {
                 //without this, sdcard will show as if it was being partitioned even if no external sd inserted!
                //ensure_path_mounted(volume) could be added to static void partition_sdcard(const char* volume) function
                ui_print("Can't mount %s\n", other_sd);
                break;
            }
            if (can_partition(other_sd)) {
                partition_sdcard(other_sd);
            }
            break;

In original code, if no second sdcard is inserted, it will show as if you can partition it and partitioning was successful while it is not

@PhilZ-cwm6
Copy link

Choose a reason for hiding this comment

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

edited code above for fix

@koush
Copy link
Author

@koush koush commented on d3efd1f Nov 26, 2012

Choose a reason for hiding this comment

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

Provide a diff please

@koush
Copy link
Author

@koush koush commented on d3efd1f Nov 26, 2012

Choose a reason for hiding this comment

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

the bug may be in get_filtered_menu_selection. that should show menu items even if previous items were NULLed out.

@PhilZ-cwm6
Copy link

Choose a reason for hiding this comment

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

If by diff you mean difference, here it is (I'd edit inline but it seems commits that way are not accepted...)

Original code in show_advanced_menu()

static char* list[] = { "Reboot Recovery",
                        "Wipe Dalvik Cache",
                        "Wipe Battery Stats",
                        "Report Error",
                        "Key Test",
                        "Show log",
                        "Fix Permissions",
                        "Partition sdcard",
                        "Partition External sdcard",
                        "Partition Internal sdcard",
                        NULL
};

if (!can_partition("/sdcard")) {
    list[7] = NULL;
}
if (!can_partition("/external_sd")) {
    list[8] = NULL;
}
if (!can_partition("/emmc")) {
    list[9] = NULL;
}

Becomes:

static char* list[] = { "Reboot Recovery",
                        "Wipe Dalvik Cache",
                        "Wipe Battery Stats",
                        "Report Error",
                        "Key Test",
                        "Show log",
                        "Fix Permissions",
                        NULL,
                        NULL,
                        NULL
};

char *other_sd = NULL;
if (volume_for_path("/emmc") != NULL) {
    other_sd = "/emmc";
    list[7] = "Partition External sdcard";
    list[8] = "Partition Internal sdcard";
} else if (volume_for_path("/external_sd") != NULL) {
    other_sd = "/external_sd";
    list[7] = "Partition Internal sdcard";
    list[8] = "Partition External sdcard";
}

Then, in same void show_advanced_menu():

Original is:

        case 8:
            partition_sdcard("/sdcard");
            break;
        case 9:
            partition_sdcard("/external_sd");
            break;
        case 10:
            partition_sdcard("/emmc");
            break;

Proposed changes that do work:

        case 7:
            //without this, we have a success partitioning log even on a non existing external sdcard
            //would be more usefull to add thi scheck in partition_sdcard ()
            //as a quick fix, I add it here
            if (ensure_path_mounted("/sdcard") != 0) {
                ui_print("Can't mount /sdcard\n");
                break;
            }
            if (can_partition("/sdcard")) {
                //no need to unmount, sdparted script does it well
                partition_sdcard("/sdcard");
            }
            break;
        case 8:
            if (ensure_path_mounted(other_sd) != 0) {
                ui_print("Can't mount %s\n", other_sd);
                break;
            }
            if (can_partition("/other_sd")) {
                partition_sdcard("/other_sd");
            }
            break;

This will also fix the bug where we get a successful formatting output log on a non existing external sdcard because of the missing ensure_path_mounted() in partition_sdcard() function
See inline comments on my proposed edit

This will provide a clean fix until the bug source is identified as you suggested (get_filtered_menu_selection) or fflush(stdout)?

@PhilZ-cwm6
Copy link

Choose a reason for hiding this comment

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

cannot git diff, but here's a diff link from my repo
PhilZ-cwm6/philz_touch_cwm6@fddbc56

Just list [8] and [9] will be for you [7] and [8]

Please sign in to comment.