All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 1/2] fastboot: getvar_partition_{type, size}: Sanitize arguments
@ 2019-03-19 22:57 Eugeniu Rosca
  2019-03-19 22:57 ` [U-Boot] [PATCH 2/2] fastboot: getvar_partition_type: Return 'raw' when FS-unaware Eugeniu Rosca
  2019-03-28 13:36 ` [U-Boot] [PATCH 1/2] fastboot: getvar_partition_{type, size}: Sanitize arguments Eugeniu Rosca
  0 siblings, 2 replies; 5+ messages in thread
From: Eugeniu Rosca @ 2019-03-19 22:57 UTC (permalink / raw)
  To: u-boot

Correct usage of "fastboot getvar_partition_{type,size}" is:

host $> fastboot getvar  partition-size:misc
partition-size:misc: 0x0000000000000400
Finished. Total time: 0.214s

When the partition name is omitted, current behavior is:

host $> fastboot getvar  partition-size
getvar:partition-size FAILED (remote: partition not found)
finished. total time: 0.005s
host $> fastboot getvar  partition-size:
getvar:partition-size: FAILED (remote: partition not found)
finished. total time: 0.006s
host $> fastboot getvar  partition-type
getvar:partition-type FAILED (remote: partition not found)
finished. total time: 0.005s
host $> fastboot getvar  partition-type:
getvar:partition-type: FAILED (remote: partition not found)
finished. total time: 0.006s

Tell the user the real cause of command failure:

host $> fastboot getvar  partition-size
getvar:partition-size FAILED (remote: missing partition name)
Finished. Total time: 0.003s
host $> fastboot getvar  partition-size:
getvar:partition-size: FAILED (remote: missing partition name)
Finished. Total time: 0.003s
host $> fastboot getvar  partition-type
getvar:partition-type FAILED (remote: missing partition name)
Finished. Total time: 0.003s
host $> fastboot getvar  partition-type:
getvar:partition-type: FAILED (remote: missing partition name)
Finished. Total time: 0.003s

Fixes: f73a7df984a9 ("net: fastboot: Merge AOSP UDP fastboot")
Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
---
 drivers/fastboot/fb_getvar.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c
index 4d264c985d7e..28e3e2fa1619 100644
--- a/drivers/fastboot/fb_getvar.c
+++ b/drivers/fastboot/fb_getvar.c
@@ -144,6 +144,11 @@ static void getvar_partition_type(char *part_name, char *response)
 	struct blk_desc *dev_desc;
 	disk_partition_t part_info;
 
+	if (!part_name || !strcmp(part_name, "")) {
+		fastboot_fail("missing partition name", response);
+		return;
+	}
+
 	r = fastboot_mmc_get_part_info(part_name, &dev_desc, &part_info,
 				       response);
 	if (r >= 0) {
@@ -162,6 +167,11 @@ static void getvar_partition_size(char *part_name, char *response)
 	int r;
 	size_t size;
 
+	if (!part_name || !strcmp(part_name, "")) {
+		fastboot_fail("missing partition name", response);
+		return;
+	}
+
 #if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)
 	struct blk_desc *dev_desc;
 	disk_partition_t part_info;
-- 
2.21.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [U-Boot] [PATCH 2/2] fastboot: getvar_partition_type: Return 'raw' when FS-unaware
  2019-03-19 22:57 [U-Boot] [PATCH 1/2] fastboot: getvar_partition_{type, size}: Sanitize arguments Eugeniu Rosca
@ 2019-03-19 22:57 ` Eugeniu Rosca
  2019-03-25 11:00   ` Eugeniu Rosca
  2019-03-26 20:21   ` Eugeniu Rosca
  2019-03-28 13:36 ` [U-Boot] [PATCH 1/2] fastboot: getvar_partition_{type, size}: Sanitize arguments Eugeniu Rosca
  1 sibling, 2 replies; 5+ messages in thread
From: Eugeniu Rosca @ 2019-03-19 22:57 UTC (permalink / raw)
  To: u-boot

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(-)

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);
 	}
-- 
2.21.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [U-Boot] [PATCH 2/2] fastboot: getvar_partition_type: Return 'raw' when FS-unaware
  2019-03-19 22:57 ` [U-Boot] [PATCH 2/2] fastboot: getvar_partition_type: Return 'raw' when FS-unaware Eugeniu Rosca
@ 2019-03-25 11:00   ` Eugeniu Rosca
  2019-03-26 20:21   ` Eugeniu Rosca
  1 sibling, 0 replies; 5+ messages in thread
From: Eugeniu Rosca @ 2019-03-25 11:00 UTC (permalink / raw)
  To: u-boot

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.

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [U-Boot] [PATCH 2/2] fastboot: getvar_partition_type: Return 'raw' when FS-unaware
  2019-03-19 22:57 ` [U-Boot] [PATCH 2/2] fastboot: getvar_partition_type: Return 'raw' when FS-unaware Eugeniu Rosca
  2019-03-25 11:00   ` Eugeniu Rosca
@ 2019-03-26 20:21   ` Eugeniu Rosca
  1 sibling, 0 replies; 5+ messages in thread
From: Eugeniu Rosca @ 2019-03-26 20:21 UTC (permalink / raw)
  To: u-boot

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.

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [U-Boot] [PATCH 1/2] fastboot: getvar_partition_{type, size}: Sanitize arguments
  2019-03-19 22:57 [U-Boot] [PATCH 1/2] fastboot: getvar_partition_{type, size}: Sanitize arguments Eugeniu Rosca
  2019-03-19 22:57 ` [U-Boot] [PATCH 2/2] fastboot: getvar_partition_type: Return 'raw' when FS-unaware Eugeniu Rosca
@ 2019-03-28 13:36 ` Eugeniu Rosca
  1 sibling, 0 replies; 5+ messages in thread
From: Eugeniu Rosca @ 2019-03-28 13:36 UTC (permalink / raw)
  To: u-boot

Superseded by https://patchwork.ozlabs.org/patch/1068195/ ("[U-Boot,v2]
fastboot: Improve error reporting on 'getvar partition-{size, type}'")

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-03-28 13:36 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-19 22:57 [U-Boot] [PATCH 1/2] fastboot: getvar_partition_{type, size}: Sanitize arguments Eugeniu Rosca
2019-03-19 22:57 ` [U-Boot] [PATCH 2/2] fastboot: getvar_partition_type: Return 'raw' when FS-unaware Eugeniu Rosca
2019-03-25 11:00   ` Eugeniu Rosca
2019-03-26 20:21   ` Eugeniu Rosca
2019-03-28 13:36 ` [U-Boot] [PATCH 1/2] fastboot: getvar_partition_{type, size}: Sanitize arguments Eugeniu Rosca

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.