Error handling broken for block devices

Error handling broken for block devices

[Wolfgang Denk]

Hi,

I noticed that block device commands like "ls" fail to give any error
messages in some cases.  For example, something like

	-> ls FOOBAR 1:1 /
	->

will terminate without the slightest hint that FOOBAR is some device
type it does not know.

Following the call chain I see this:

"ls" command -> do_ls (...) -> fs_set_blk_dev(...) -> blk_get_device_part_str()

"disk/part.c":

431 int blk_get_device_part_str(const char *ifname, const char *dev_part_str,
432                              struct blk_desc **dev_desc,
433                              disk_partition_t *info, int allow_whole_dev)
...
450         if (0 == strcmp(ifname, "hostfs")) {
...
466         }
...
474         if (0 == strcmp(ifname, "ubi")) {
...
488         }
513         /* Look up the device */
514         dev = blk_get_device_by_str(ifname, dev_str, dev_desc);
515         if (dev < 0)
516                 goto cleanup;

No error messages get printed here, but I think these are mandatory
in such a case.


Stephen, this code was added as part of your commit:

commit 10a37fd7a40826c43a63591855346adf1a1ac02d
Author: Stephen Warren <swarren@nvidia.com>
Date:   Fri Sep 21 09:50:57 2012 +0000

    disk: get_device_and_partition() "auto" partition and cleanup


Can you think of any strong reason _not_ to issue a proper error
message here?

[From the user perspective it would also be nice to have a way to
find out which device types are actually supported in a given U-Boot
binary.  Or is there such a tool?]

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
It's not an optical illusion, it just looks like one.   -- Phil White

Error handling broken for block devices

[Stephen Warren]

On 1/20/20 9:40 AM, Wolfgang Denk wrote:
> Hi,
> 
> I noticed that block device commands like "ls" fail to give any error
> messages in some cases.  For example, something like
> 
> 	-> ls FOOBAR 1:1 /
> 	->
> 
> will terminate without the slightest hint that FOOBAR is some device
> type it does not know.
...
> No error messages get printed here, but I think these are mandatory
> in such a case.
> 
> 
> Stephen, this code was added as part of your commit:
> 
> commit 10a37fd7a40826c43a63591855346adf1a1ac02d
> Author: Stephen Warren <swarren@nvidia.com>
> Date:   Fri Sep 21 09:50:57 2012 +0000
> 
>     disk: get_device_and_partition() "auto" partition and cleanup
> 
> 
> Can you think of any strong reason _not_ to issue a proper error
> message here?

No, this path should definitely print an error message somewhere.

It's possible that when this command was originally implemented, some
nested function printed an error, such that the command implementation
itself didn't too, to avoid duplication. Or maybe I just forgot to add
the error print.