Re: [PATCH v2 4/7] cmd: avb: rework prints

13 Feb 2024

Hi Igor,
Thank you for the patch.
On ven., févr. 09, 2024 at 20:20, Igor Opaniuk igor.opaniuk@foundries.io wrote:
...
From: Igor Opaniuk igor.opaniuk@gmail.com
Simplify and add more context for prints where it's needed.
Signed-off-by: Igor Opaniuk igor.opaniuk@gmail.com
Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com
...

Changes in v2:

Drop AVB_OPS_CHECK macro and leave previous check

cmd/avb.c | 123 +++++++++++++++++++++++++++++++++---------------------
 1 file changed, 75 insertions(+), 48 deletions(-)

diff --git a/cmd/avb.c b/cmd/avb.c
index ce8b63873f2..62a3ee18e7f 100644
--- a/cmd/avb.c
+++ b/cmd/avb.c
@@ -11,6 +11,7 @@
 #include <mmc.h>
#define AVB_BOOTARGS	"avb_bootargs"



static struct AvbOps *avb_ops;
int do_avb_init(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
@@ -28,8 +29,10 @@ int do_avb_init(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
   avb_ops = avb_ops_alloc(mmc_dev);
   if (avb_ops)
   	return CMD_RET_SUCCESS;

else
printf("Can't allocate AvbOps");





printf("Failed to initialize avb2\n");


printf("Failed to initialize AVB\n");
return CMD_RET_FAILURE;


}
@@ -41,10 +44,11 @@ int do_avb_read_part(struct cmd_tbl *cmdtp, int flag, int argc,
   s64 offset;
   size_t bytes, bytes_read = 0;
   void *buffer;

int ret;
if (!avb_ops) {



printf("AVB 2.0 is not initialized, please run 'avb init'\n");


return CMD_RET_USAGE;




printf("AVB is not initialized, please run 'avb init <id>'\n");


return CMD_RET_FAILURE;

}
if (argc != 5)


@@ -55,14 +59,15 @@ int do_avb_read_part(struct cmd_tbl *cmdtp, int flag, int argc,
   bytes = hextoul(argv[3], NULL);
   buffer = (void *)hextoul(argv[4], NULL);

if (avb_ops->read_from_partition(avb_ops, part, offset, bytes,
			 buffer, &bytes_read) ==


			 AVB_IO_RESULT_OK) {




ret = avb_ops->read_from_partition(avb_ops, part, offset,
			   bytes, buffer, &bytes_read);


if (ret == AVB_IO_RESULT_OK) {
printf("Read %zu bytes\n", bytes_read);
return CMD_RET_SUCCESS;
}


printf("Failed to read from partition\n");


printf("Failed to read from partition '%s', err = %d\n",

      part, ret);


return CMD_RET_FAILURE;


}
@@ -74,10 +79,11 @@ int do_avb_read_part_hex(struct cmd_tbl *cmdtp, int flag, int argc,
   s64 offset;
   size_t bytes, bytes_read = 0;
   char *buffer;

int ret;
if (!avb_ops) {



printf("AVB 2.0 is not initialized, please run 'avb init'\n");


return CMD_RET_USAGE;




printf("AVB is not initialized, please run 'avb init <id>'\n");


return CMD_RET_FAILURE;

}
if (argc != 4)


@@ -94,8 +100,9 @@ int do_avb_read_part_hex(struct cmd_tbl *cmdtp, int flag, int argc,
   }
   memset(buffer, 0, bytes);

if (avb_ops->read_from_partition(avb_ops, part, offset, bytes, buffer,
			 &bytes_read) == AVB_IO_RESULT_OK) {




ret = avb_ops->read_from_partition(avb_ops, part, offset,
			   bytes, buffer, &bytes_read);


if (ret == AVB_IO_RESULT_OK) {
printf("Requested %zu, read %zu bytes\n", bytes, bytes_read);
printf("Data: ");
for (int i = 0; i < bytes_read; i++)

@@ -107,7 +114,8 @@ int do_avb_read_part_hex(struct cmd_tbl *cmdtp, int flag, int argc,
   	return CMD_RET_SUCCESS;
   }

printf("Failed to read from partition\n");


printf("Failed to read from partition '%s', err = %d\n",

      part, ret);


free(buffer);
 return CMD_RET_FAILURE;


@@ -120,9 +128,10 @@ int do_avb_write_part(struct cmd_tbl *cmdtp, int flag, int argc,
   s64 offset;
   size_t bytes;
   void *buffer;

int ret;
if (!avb_ops) {



printf("AVB 2.0 is not initialized, run 'avb init' first\n");




printf("AVB is not initialized, please run 'avb init <id>'\n");

return CMD_RET_FAILURE;
}

@@ -134,13 +143,15 @@ int do_avb_write_part(struct cmd_tbl *cmdtp, int flag, int argc,
   bytes = hextoul(argv[3], NULL);
   buffer = (void *)hextoul(argv[4], NULL);

if (avb_ops->write_to_partition(avb_ops, part, offset, bytes, buffer) ==
   AVB_IO_RESULT_OK) {




ret = avb_ops->write_to_partition(avb_ops, part, offset,
			  bytes, buffer);


if (ret == AVB_IO_RESULT_OK) {
printf("Wrote %zu bytes\n", bytes);
return CMD_RET_SUCCESS;
}


printf("Failed to write in partition\n");


printf("Failed to write in partition '%s', err = %d\n",

      part, ret);


return CMD_RET_FAILURE;


}
@@ -150,9 +161,10 @@ int do_avb_read_rb(struct cmd_tbl *cmdtp, int flag, int argc,
 {
   size_t index;
   u64 rb_idx;

int ret;
if (!avb_ops) {



printf("AVB 2.0 is not initialized, run 'avb init' first\n");




printf("AVB is not initialized, please run 'avb init <id>'\n");

return CMD_RET_FAILURE;
}

@@ -161,13 +173,14 @@ int do_avb_read_rb(struct cmd_tbl *cmdtp, int flag, int argc,
index = (size_t)hextoul(argv[1], NULL);

if (avb_ops->read_rollback_index(avb_ops, index, &rb_idx) ==
   AVB_IO_RESULT_OK) {




ret = avb_ops->read_rollback_index(avb_ops, index, &rb_idx);
if (ret == AVB_IO_RESULT_OK) {
printf("Rollback index: %llx\n", rb_idx);
return CMD_RET_SUCCESS;
}


printf("Failed to read rollback index\n");


printf("Failed to read rollback index id = %zu, err = %d\n",

      index, ret);


return CMD_RET_FAILURE;


}
@@ -177,9 +190,10 @@ int do_avb_write_rb(struct cmd_tbl *cmdtp, int flag, int argc,
 {
   size_t index;
   u64 rb_idx;

int ret;
if (!avb_ops) {



printf("AVB 2.0 is not initialized, run 'avb init' first\n");




printf("AVB is not initialized, please run 'avb init <id>'\n");

return CMD_RET_FAILURE;
}

@@ -189,11 +203,12 @@ int do_avb_write_rb(struct cmd_tbl *cmdtp, int flag, int argc,
   index = (size_t)hextoul(argv[1], NULL);
   rb_idx = hextoul(argv[2], NULL);

if (avb_ops->write_rollback_index(avb_ops, index, rb_idx) ==
   AVB_IO_RESULT_OK)




ret = avb_ops->write_rollback_index(avb_ops, index, rb_idx);
if (ret == AVB_IO_RESULT_OK)
return CMD_RET_SUCCESS;


printf("Failed to write rollback index\n");


printf("Failed to write rollback index id = %zu, err = %d\n",

      index, ret);


return CMD_RET_FAILURE;


}
@@ -203,9 +218,10 @@ int do_avb_get_uuid(struct cmd_tbl *cmdtp, int flag,
 {
   const char *part;
   char buffer[UUID_STR_LEN + 1];

int ret;
if (!avb_ops) {



printf("AVB 2.0 is not initialized, run 'avb init' first\n");




printf("AVB is not initialized, please run 'avb init <id>'\n");

return CMD_RET_FAILURE;
}

@@ -214,14 +230,16 @@ int do_avb_get_uuid(struct cmd_tbl *cmdtp, int flag,
part = argv[1];

if (avb_ops->get_unique_guid_for_partition(avb_ops, part, buffer,
				   UUID_STR_LEN + 1) ==


				   AVB_IO_RESULT_OK) {




ret = avb_ops->get_unique_guid_for_partition(avb_ops, part,
				     buffer,


				     UUID_STR_LEN + 1);


if (ret == AVB_IO_RESULT_OK) {
printf("'%s' UUID: %s\n", part, buffer);
return CMD_RET_SUCCESS;
}


printf("Failed to read UUID\n");


printf("Failed to read partition '%s' UUID, err = %d\n",

      part, ret);


return CMD_RET_FAILURE;


}
@@ -235,12 +253,13 @@ int do_avb_verify_part(struct cmd_tbl *cmdtp, int flag,
   char *cmdline;
   char *extra_args;
   char *slot_suffix = "";

int ret;
bool unlocked = false;
int res = CMD_RET_FAILURE;
if (!avb_ops) {



printf("AVB 2.0 is not initialized, run 'avb init' first\n");




printf("AVB is not initialized, please run 'avb init <id>'\n");

return CMD_RET_FAILURE;
}

@@ -253,9 +272,10 @@ int do_avb_verify_part(struct cmd_tbl *cmdtp, int flag,
   printf("## Android Verified Boot 2.0 version %s\n",
          avb_version_string());

if (avb_ops->read_is_device_unlocked(avb_ops, &unlocked) !=
   AVB_IO_RESULT_OK) {


printf("Can't determine device lock state.\n");




ret = avb_ops->read_is_device_unlocked(avb_ops, &unlocked);
if (ret != AVB_IO_RESULT_OK) {
printf("Can't determine device lock state, err = %d\n",


       ret);

return CMD_RET_FAILURE;
}

@@ -302,10 +322,10 @@ int do_avb_verify_part(struct cmd_tbl *cmdtp, int flag,
   	printf("Corrupted dm-verity metadata detected\n");
   	break;
   case AVB_SLOT_VERIFY_RESULT_ERROR_UNSUPPORTED_VERSION:

printf("Unsupported version avbtool was used\n");




printf("Unsupported version of avbtool was used\n");

break;
case AVB_SLOT_VERIFY_RESULT_ERROR_ROLLBACK_INDEX:


printf("Checking rollback index failed\n");




printf("Rollback index check failed\n");

break;
case AVB_SLOT_VERIFY_RESULT_ERROR_PUBLIC_KEY_REJECTED:
printf("Public key was rejected\n");

@@ -324,9 +344,10 @@ int do_avb_is_unlocked(struct cmd_tbl *cmdtp, int flag,
   	       int argc, char *const argv[])
 {
   bool unlock;

int ret;
if (!avb_ops) {



printf("AVB not initialized, run 'avb init' first\n");




printf("AVB is not initialized, please run 'avb init <id>'\n");

return CMD_RET_FAILURE;
}

@@ -335,13 +356,14 @@ int do_avb_is_unlocked(struct cmd_tbl *cmdtp, int flag,
   	return CMD_RET_USAGE;
   }

if (avb_ops->read_is_device_unlocked(avb_ops, &unlock) ==
   AVB_IO_RESULT_OK) {




ret = avb_ops->read_is_device_unlocked(avb_ops, &unlock);
if (ret == AVB_IO_RESULT_OK) {
printf("Unlocked = %d\n", unlock);
return CMD_RET_SUCCESS;
}


printf("Can't determine device lock state.\n");


printf("Can't determine device lock state, err = %d\n",

      ret);


return CMD_RET_FAILURE;


}
@@ -354,9 +376,10 @@ int do_avb_read_pvalue(struct cmd_tbl *cmdtp, int flag, int argc,
   size_t bytes_read;
   void *buffer;
   char *endp;

int ret;
if (!avb_ops) {



printf("AVB 2.0 is not initialized, run 'avb init' first\n");




printf("AVB is not initialized, please run 'avb init <id>'\n");

return CMD_RET_FAILURE;
}

@@ -372,15 +395,16 @@ int do_avb_read_pvalue(struct cmd_tbl *cmdtp, int flag, int argc,
   if (!buffer)
   	return CMD_RET_FAILURE;

if (avb_ops->read_persistent_value(avb_ops, name, bytes, buffer,
			   &bytes_read) == AVB_IO_RESULT_OK) {




ret = avb_ops->read_persistent_value(avb_ops, name, bytes,
			     buffer, &bytes_read);


if (ret == AVB_IO_RESULT_OK) {
printf("Read %zu bytes, value = %s\n", bytes_read,
      (char *)buffer);
free(buffer);
return CMD_RET_SUCCESS;
}


printf("Failed to read persistent value\n");


printf("Failed to read persistent value, err = %d\n", ret);
free(buffer);


@@ -392,9 +416,10 @@ int do_avb_write_pvalue(struct cmd_tbl *cmdtp, int flag, int argc,
 {
   const char *name;
   const char *value;

int ret;
if (!avb_ops) {



printf("AVB 2.0 is not initialized, run 'avb init' first\n");




printf("AVB is not initialized, please run 'avb init <id>'\n");

return CMD_RET_FAILURE;
}

@@ -404,14 +429,16 @@ int do_avb_write_pvalue(struct cmd_tbl *cmdtp, int flag, int argc,
   name = argv[1];
   value = argv[2];

if (avb_ops->write_persistent_value(avb_ops, name, strlen(value) + 1,
			    (const uint8_t *)value) ==


   AVB_IO_RESULT_OK) {




ret = avb_ops->write_persistent_value(avb_ops, name,
			      strlen(value) + 1,


			      (const uint8_t *)value);


if (ret == AVB_IO_RESULT_OK) {
printf("Wrote %zu bytes\n", strlen(value) + 1);
return CMD_RET_SUCCESS;
}


printf("Failed to write persistent value\n");


printf("Failed to write persistent value `%s` = `%s`, err = %d\n",

      name, value, ret);


return CMD_RET_FAILURE;


}
2.34.1

    