diff mbox

[U-Boot,v2,3/3] common: cmd_part: Error prints on failures

Message ID 1434184697-23410-4-git-send-email-contact@paulk.fr
State Superseded
Delegated to: Tom Rini
Headers show

Commit Message

Paul Kocialkowski June 13, 2015, 8:38 a.m. UTC
When a failure occurs when selecting the device or partition, the user should be
notified through an error print.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 common/cmd_part.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

Comments

Stephen Warren June 15, 2015, 3 p.m. UTC | #1
On 06/13/2015 02:38 AM, Paul Kocialkowski wrote:
> When a failure occurs when selecting the device or partition, the user should be
> notified through an error print.

> diff --git a/common/cmd_part.c b/common/cmd_part.c

> @@ -38,8 +38,10 @@ static int do_part_uuid(int argc, char * const argv[])
>   		return CMD_RET_USAGE;
>
>   	part = get_device_and_partition(argv[0], argv[1], &dev_desc, &info, 0);
> -	if (part < 0)
> +	if (part < 0) {
> +		error("Invalid device and/or partition\n");

A very quick look at the implementation of get_device_and_partition() 
(and all the other relevant functions for this patch) implies the 
implementation already prints an error message. If you found a case 
where that isn't true, I think those functions should be fixed, not all 
their callers.
Paul Kocialkowski June 15, 2015, 7:25 p.m. UTC | #2
Le lundi 15 juin 2015 à 09:00 -0600, Stephen Warren a écrit :
> On 06/13/2015 02:38 AM, Paul Kocialkowski wrote:
> > When a failure occurs when selecting the device or partition, the user should be
> > notified through an error print.
> 
> > diff --git a/common/cmd_part.c b/common/cmd_part.c
> 
> > @@ -38,8 +38,10 @@ static int do_part_uuid(int argc, char * const argv[])
> >   		return CMD_RET_USAGE;
> >
> >   	part = get_device_and_partition(argv[0], argv[1], &dev_desc, &info, 0);
> > -	if (part < 0)
> > +	if (part < 0) {
> > +		error("Invalid device and/or partition\n");
> 
> A very quick look at the implementation of get_device_and_partition() 
> (and all the other relevant functions for this patch) implies the 
> implementation already prints an error message. If you found a case 
> where that isn't true, I think those functions should be fixed, not all 
> their callers.

You're right, I'll just drop that patch from the series.

Thanks!
diff mbox

Patch

diff --git a/common/cmd_part.c b/common/cmd_part.c
index 7212ba6..ff9bc07 100644
--- a/common/cmd_part.c
+++ b/common/cmd_part.c
@@ -38,8 +38,10 @@  static int do_part_uuid(int argc, char * const argv[])
 		return CMD_RET_USAGE;
 
 	part = get_device_and_partition(argv[0], argv[1], &dev_desc, &info, 0);
-	if (part < 0)
+	if (part < 0) {
+		error("Invalid device and/or partition\n");
 		return 1;
+	}
 
 	if (argc > 2)
 		setenv(argv[2], info.uuid);
@@ -82,8 +84,10 @@  static int do_part_list(int argc, char * const argv[])
 	}
 
 	ret = get_device(argv[0], argv[1], &desc);
-	if (ret < 0)
+	if (ret < 0) {
+		error("Invalid device\n");
 		return 1;
+	}
 
 	if (var != NULL) {
 		int p;
@@ -127,12 +131,16 @@  static int do_part_start(int argc, char * const argv[])
 	part = simple_strtoul(argv[2], NULL, 0);
 
 	ret = get_device(argv[0], argv[1], &desc);
-	if (ret < 0)
+	if (ret < 0) {
+		error("Invalid device\n");
 		return 1;
+	}
 
 	err = get_partition_info(desc, part, &info);
-	if (err)
+	if (err) {
+		error("Invalid partition number\n");
 		return 1;
+	}
 
 	snprintf(buf, sizeof(buf), "0x%lx", info.start);
 	setenv(argv[3], buf);
@@ -155,12 +163,16 @@  static int do_part_size(int argc, char * const argv[])
 	part = simple_strtoul(argv[2], NULL, 0);
 
 	ret = get_device(argv[0], argv[1], &desc);
-	if (ret < 0)
+	if (ret < 0) {
+		error("Invalid device\n");
 		return 1;
+	}
 
 	err = get_partition_info(desc, part, &info);
-	if (err)
+	if (err) {
+		error("Invalid partition number\n");
 		return 1;
+	}
 
 	snprintf(buf, sizeof(buf), "0x%lx", info.size);
 	setenv(argv[3], buf);