UBIFS: Improve error message when reading superblock failed

PATCH New Bernhard Walle 6 participants 11 messages Feb 9, 2012 – Aug 9, 2012 UBI file system (UBIFS)

Share & tools

Bernhard Walle

author

2012-02-09 17:15 UTC #0 lore raw

PATCH 1/1 UBIFS: Improve error message when reading superblock failed

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 <[email protected]>

---
 fs/ubifs/super.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

fs/ubifs/super.c +1 -1

index 26b48f0..0b1440b 100644
@@ -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;
 	}

Signature · 1 line

1.7.9

Detlev Zundel

2012-02-17 14:15 UTC #1 lore raw

PATCH 1/1 UBIFS: Improve error message when reading superblock failed

Hi Bernhard,

Quoted 15 lines

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 <[email protected]>
---
 fs/ubifs/super.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
fs/ubifs/super.c +0 -0
index 26b48f0..0b1440b 100644

fs/ubifs/super.c int ubifs_mount(char *vol_name) · line 1196

Quoted 7 lines

fs/ubifs/super.c +1 -1

@@ -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

Signature · 7 lines

The fact is, volatile on data structures is a bug. It's a wart in the C 
language. It shouldn't be used.
                                   -- Linus Torvalds 
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

Bernhard Walle

author

2012-02-17 14:31 UTC #2 lore raw

PATCH 1/1 UBIFS: Improve error message when reading superblock failed

Hi Detlev,

Quoted 14 lines · Detlev Zundel

fs/ubifs/super.c int ubifs_mount(char *vol_name) · line 1196
Quoted 9 lines
fs/ubifs/super.c +1 -1
@@ -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

2012-02-17 15:00 UTC #3 lore raw

PATCH 1/1 UBIFS: Improve error message when reading superblock failed

Hi Bernhard,

Quoted 18 lines

Hi Detlev,
Quoted 14 lines · Detlev Zundel
fs/ubifs/super.c int ubifs_mount(char *vol_name) · line 1196
Quoted 9 lines
fs/ubifs/super.c +1 -1
@@ -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.

Quoted 3 lines

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

Signature · 9 lines

Basically,  Barnes & Noble separates things by how old they are  -- current
stuff is "Fiction", stuff from 20 years ago is "Literature", stuff from 100
years ago is "Classics", stuff from 400 years ago is "Shakespeare" [..] and
stuff from 2000 years ago is "History".
                     -- James "Kibo" Parry in <[email protected]>
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

Bernhard Walle

author

2012-02-20 08:44 UTC #4 lore raw

PATCH 1/1 UBIFS: Improve error message when reading superblock failed

In reply to Bernhard Walle

Signed-off-by Bernhard Walle <[email protected]>

---
v2: Print the non-negated error value.

 fs/ubifs/super.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

fs/ubifs/super.c +1 -1

index 26b48f0..e6c02f5 100644
@@ -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;
 	}

Signature · 1 line

1.7.9.1

Albert ARIBAUD

2012-02-20 08:59 UTC #5 lore raw

PATCH 1/1 UBIFS: Improve error message when reading superblock failed

Hi Bernard,

Le 20/02/2012 09:44, Bernhard Walle a ?crit :

Quoted 17 lines

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<[email protected]>
---
v2: Print the non-negated error value.
  fs/ubifs/super.c |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
fs/ubifs/super.c +0 -0
index 26b48f0..e6c02f5 100644

fs/ubifs/super.c int ubifs_mount(char *vol_name) · line 1197

Quoted 8 lines

fs/ubifs/super.c +1 -1

@@ -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;
  	}

Dry numbers as error messages are better than no error messages but only marginally IMO. Isn't there a way to emit a readable message re malloc instead of emitting an int value?

Amicalement,

Signature · 1 line

Albert.

Bernhard Walle

author

2012-02-20 09:11 UTC #6 lore raw

PATCH 1/1 UBIFS: Improve error message when reading superblock failed

Am 20.02.2012 09:59, schrieb Albert ARIBAUD:

Quoted 4 lines

Dry numbers as error messages are better than no error messages but only marginally IMO. Isn't there a way to emit a readable message re malloc instead of emitting an int value?

Well, I'm not familiar with the u-boot codebase. Does u-boot have a strerror table? How is it handled on other places? Should an error message printed directly before returning -ENOMEM?

Regards,
Bernhard

Albert ARIBAUD

2012-02-20 10:01 UTC #7 lore raw

PATCH 1/1 UBIFS: Improve error message when reading superblock failed

Hi Bernhard,

Le 20/02/2012 10:11, Bernhard Walle a ?crit :

Quoted 9 lines

Am 20.02.2012 09:59, schrieb Albert ARIBAUD:
Quoted 4 lines
Dry numbers as error messages are better than no error messages but only marginally IMO. Isn't there a way to emit a readable message re malloc instead of emitting an int value?
Well, I'm not familiar with the u-boot codebase. Does u-boot have a strerror table? How is it handled on other places? Should an error message printed directly before returning -ENOMEM?

I don't think there is an strerror() API. But at least, if the newly printed int uses errno values, then instead of '... %d!", you could print "... errno=%d", which would give some hint to the reader.

Quoted 2 lines

Regards,
Bernhard

Amicalement,

Signature · 1 line

Albert.

Thomas Weber

2012-04-02 11:58 UTC #8 lore raw

PATCH 1/1 UBIFS: Improve error message when reading superblock failed

From: Bernhard Walle <[email protected]>

Signed-off-by Bernhard Walle <[email protected]>

Use ubifs_err instead of printf.
Add "errno=%d" in output as suggested by Albert Aribaud.

Signed-off-by Thomas Weber <[email protected]>

---
 fs/ubifs/super.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

fs/ubifs/super.c +1 -1

index 26b48f0..30ccd98 100644
@@ -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);
+		ubifs_err("Error reading superblock on volume '%s' errno=%d!\n", name, ret);
 		return -1;
 	}

Signature · 1 line

1.7.8.5

Thomas Weber

2012-05-15 20:33 UTC #9 lore raw

PATCH 1/1 UBIFS: Improve error message when reading superblock failed

Ccing Albert

Thomas

Quoted 29 lines · Thomas Weber

From: Bernhard Walle <[email protected]>
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 <[email protected]>
Use ubifs_err instead of printf.
Add "errno=%d" in output as suggested by Albert Aribaud.
Signed-off-by Thomas Weber <[email protected]>
---
 fs/ubifs/super.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
fs/ubifs/super.c +1 -1
index 26b48f0..30ccd98 100644
@@ -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);
+		ubifs_err("Error reading superblock on volume '%s' errno=%d!\n", name, ret);
 		return -1;
 	}
 

Wolfgang Denk

Applied

2012-08-09 20:12 UTC #10 lore raw

PATCH 1/1 UBIFS: Improve error message when reading superblock failed

In reply to Thomas Weber

Dear Thomas Weber,

Quoted 7 lines · In message <[email protected]> you wrote:

From: Bernhard Walle <[email protected]>
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.

Quoted 8 lines

Signed-off-by Bernhard Walle <[email protected]>
Use ubifs_err instead of printf.
Add "errno=%d" in output as suggested by Albert Aribaud.
Signed-off-by Thomas Weber <[email protected]>
---
 fs/ubifs/super.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Applied, thanks.

Stefan, I hope this is OK with you.

Best regards,

Wolfgang Denk

Signature · 3 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
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

Quoted 1 line

Is there a way to determine Yesterday's date using Unix utilities?

echo "what is yesterday's date?" | /bin/mail root -- Randal L. Schwartz in <[email protected]>