| Message ID | 20190319225706.27649-2-erosca@de.adit-jv.com |
|---|---|
| State | Superseded |
| Delegated to: | Lukasz Majewski |
| Headers | show |
| Series | [U-Boot,1/2] fastboot: getvar_partition_{type, size}: Sanitize arguments | expand |
Dear reviewers, On Tue, Mar 19, 2019 at 11:57:06PM +0100, Eugeniu Rosca wrote: > - fastboot_fail("failed to set partition", response); > + fastboot_okay("raw", response); While the issues fixed by the two patches are minor, I strongly believe they improve the user experience and I would very much appreciate any concerns/comments from you. Many thanks, Eugeniu.
On Tue, Mar 19, 2019 at 11:57:06PM +0100, Eugeniu Rosca wrote: [..] > - fastboot_fail("failed to set partition", response); > + fastboot_okay("raw", response); [..] Checking recent upstream fastboot implementation [1], I can see [2] that returning success or failure on calling 'partition-type:<part-name>' makes an impact on the internal fastboot code paths, so unfortunately I have to NAK my own patch. One idea could be calling fastboot_fail() (not fastboot_okay as this patch proposed) with an updated error message. Please, skip the patch for now. [1] https://android.googlesource.com/platform/system/core/+/eac1220fba2c [2] core (eac1220fba2c) git grep -C 1 partition-type fastboot/constants.h-#define FB_VAR_PARTITION_SIZE "partition-size" fastboot/constants.h:#define FB_VAR_PARTITION_TYPE "partition-type" fastboot/constants.h-#define FB_VAR_SLOT_SUCCESSFUL "slot-successful" -- fastboot/fastboot.cpp- fastboot/fastboot.cpp: if (fb->GetVar("partition-type:" + partition, &partition_type) != fastboot::SUCCESS) { fastboot/fastboot.cpp- errMsg = "Can't determine partition type.\n"; -- fastboot/fastboot.cpp- std::string partition_type; fastboot/fastboot.cpp: if (fb->GetVar("partition-type:" + partition, &partition_type) == fastboot::SUCCESS && fastboot/fastboot.cpp- fs_get_generator(partition_type) != nullptr) { -- fastboot/fastboot.cpp- std::string partition_type; fastboot/fastboot.cpp: if (fb->GetVar("partition-type:" + partition, &partition_type) != fastboot::SUCCESS) { fastboot/fastboot.cpp- continue; -- fastboot/fuzzy_fastboot/main.cpp- std::string partition_type; fastboot/fuzzy_fastboot/main.cpp: // getvar partition-type:super must fail for retrofit devices because the fastboot/fuzzy_fastboot/main.cpp- // partition does not exist. fastboot/fuzzy_fastboot/main.cpp: if (fb->GetVar("partition-type:super", &partition_type) == SUCCESS) { fastboot/fuzzy_fastboot/main.cpp- std::string is_logical; -- fastboot/fuzzy_fastboot/main.cpp- std::string resp; fastboot/fuzzy_fastboot/main.cpp: EXPECT_EQ(fb->GetVar("partition-type:" + part, &resp), SUCCESS); fastboot/fuzzy_fastboot/main.cpp: EXPECT_NE(allowed.find(resp), allowed.end()) << "getvar:partition-type:" + part << " was '" fastboot/fuzzy_fastboot/main.cpp- << resp << "' this is not a valid type"; -- fs_mgr/tests/adb-remount-test.sh- if [ -n "${scratch_paritition}" ]; then fs_mgr/tests/adb-remount-test.sh: fastboot_getvar partition-type:${scratch_partition} raw || fs_mgr/tests/adb-remount-test.sh- ( fastboot reboot && false) || Best regards, Eugeniu.
diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c index 28e3e2fa1619..beadf7f98e5d 100644 --- a/drivers/fastboot/fb_getvar.c +++ b/drivers/fastboot/fb_getvar.c @@ -154,7 +154,7 @@ static void getvar_partition_type(char *part_name, char *response) if (r >= 0) { r = fs_set_blk_dev_with_part(dev_desc, r); if (r < 0) - fastboot_fail("failed to set partition", response); + fastboot_okay("raw", response); else fastboot_okay(fs_get_type_name(), response); }
Only a handful of Android/fastboot partitions (e.g. /system, /vendor, /userdata) have filesystem: host $> fastboot getvar partition-type:userdata partition-type:userdata: ext4 Finished. Total time: 0.013s Most of them (/misc, /pstore, /vbmeta, /dtb{o}, /boot, etc) don't. And for the latter fastboot reports: host $> fastboot getvar partition-type:misc getvar:partition-type:misc FAILED (remote: failed to set partition) Finished. Total time: 0.219s Rather than creating pointless worries via error reporting, tell the users they are dealing with a 'raw' partition: host $> fastboot getvar partition-type:misc partition-type:misc: raw Finished. Total time: 0.017s Fixes: f73a7df984a9 ("net: fastboot: Merge AOSP UDP fastboot") Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> --- drivers/fastboot/fb_getvar.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)