diff mbox

[U-Boot] UBIFS: Improve error message when reading superblock failed

Message ID 1328807702-28146-1-git-send-email-walle@corscience.de
State Superseded
Delegated to: Stefan Roese
Headers show

Commit Message

Bernhard Walle Feb. 9, 2012, 5:15 p.m. UTC
In addition to the error message also display the error code. I had the
problem that my malloc memory was not enough (ENOMEM), and if u-boot
had displayed the error code immediately that would have saved me some
debugging.

Signed-off-by: Bernhard Walle <walle@corscience.de>
---
 fs/ubifs/super.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Detlev Zundel Feb. 17, 2012, 2:15 p.m. UTC | #1
Hi Bernhard,

> In addition to the error message also display the error code. I had the
> problem that my malloc memory was not enough (ENOMEM), and if u-boot
> had displayed the error code immediately that would have saved me some
> debugging.
>
> Signed-off-by: Bernhard Walle <walle@corscience.de>
> ---
>  fs/ubifs/super.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index 26b48f0..0b1440b 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -1191,7 +1191,7 @@ int ubifs_mount(char *vol_name)
>  	mnt = NULL;
>  	ret = ubifs_get_sb(&ubifs_fs_type, flags, name, data, mnt);
>  	if (ret) {
> -		printf("Error reading superblock on volume '%s'!\n", name);
> +		printf("Error reading superblock on volume '%s': %d!\n", name, -ret);
>  		return -1;
>  	}

I think this makes sense, but I think it would be more natural to print
the real error code, not the negative value.  I don't know how to search
for all such occurrences, but I cannot find any but a lot of sites
printing the error code as is.

Cheers
  Detlev
Bernhard Walle Feb. 17, 2012, 2:31 p.m. UTC | #2
Hi Detlev,

* Detlev Zundel <dzu@denx.de> [2012-02-17 15:15]:
> 
> > @@ -1191,7 +1191,7 @@ int ubifs_mount(char *vol_name)
> >  	mnt = NULL;
> >  	ret = ubifs_get_sb(&ubifs_fs_type, flags, name, data, mnt);
> >  	if (ret) {
> > -		printf("Error reading superblock on volume '%s'!\n", name);
> > +		printf("Error reading superblock on volume '%s': %d!\n", name, -ret);
> >  		return -1;
> >  	}
> 
> I think this makes sense, but I think it would be more natural to print
> the real error code, not the negative value.  I don't know how to search
> for all such occurrences, but I cannot find any but a lot of sites
> printing the error code as is.

well, the return value is negative, so my intention was to print the
error code as positive number. So you think we should display it as
negative number (-12 instead of 12 for ENOMEM)?


Regards,
Bernhard
Detlev Zundel Feb. 17, 2012, 3 p.m. UTC | #3
Hi Bernhard,

> Hi Detlev,
>
> * Detlev Zundel <dzu@denx.de> [2012-02-17 15:15]:
>> 
>> > @@ -1191,7 +1191,7 @@ int ubifs_mount(char *vol_name)
>> >  	mnt = NULL;
>> >  	ret = ubifs_get_sb(&ubifs_fs_type, flags, name, data, mnt);
>> >  	if (ret) {
>> > -		printf("Error reading superblock on volume '%s'!\n", name);
>> > +		printf("Error reading superblock on volume '%s': %d!\n", name, -ret);
>> >  		return -1;
>> >  	}
>> 
>> I think this makes sense, but I think it would be more natural to print
>> the real error code, not the negative value.  I don't know how to search
>> for all such occurrences, but I cannot find any but a lot of sites
>> printing the error code as is.
>
> well, the return value is negative, so my intention was to print the
> error code as positive number. So you think we should display it as
> negative number (-12 instead of 12 for ENOMEM)?

Personally I believe that any transformation in the printing can mislead
people in the search for the cause or the responsible code.

So if the error value is -12, then we should print it.  After all, the
assignment to generate that value will very likely be "return -ENOMEM"
and people will thus know what to look for.

On the other hand I am open to the consistency argument, so if every
error printing would do such a transformation then it would be better to
also do it.  But as I said, I don't know an easy grep pattern to search
for such locations and quick searches showed that I all places I found
print the error codes unmangeld.

Cheers
  Detlev
diff mbox

Patch

diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 26b48f0..0b1440b 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -1191,7 +1191,7 @@  int ubifs_mount(char *vol_name)
 	mnt = NULL;
 	ret = ubifs_get_sb(&ubifs_fs_type, flags, name, data, mnt);
 	if (ret) {
-		printf("Error reading superblock on volume '%s'!\n", name);
+		printf("Error reading superblock on volume '%s': %d!\n", name, -ret);
 		return -1;
 	}