Skip to contentSkip to conversation

EXT4: Fix number base handling of "ext4write" command

PATCH In review Wolfgang Denk 3 participants 6 messages Jan 31, 2014 – Feb 19, 2014

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 EXT4: Fix number base handling of "ext4write" command

Unlike other commands (for example, "fatwrite"), ext4write would interpret the "sizebytes" as decimal number. This is not only inconsistend and unexpected to most users, it also breaks usage like this:

	tftp ${addr} ${name}
	ext4write mmc 0:2 ${addr} ${filename} ${filesize}

Change this to use the standard notation of base 16 input format. See also commit b770e88

WARNING: this is a change to the user interface!!

Cc Uma Shankar <[email protected]>
Cc Stephen Warren <[email protected]>
---
 common/cmd_ext4.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
common/cmd_ext4.c +2 -2
index 8289d25..68b047b 100644
@@ -79,8 +79,8 @@ int do_ext4_write(cmd_tbl_t *cmdtp, int flag, int argc,
 	/* get the address in hexadecimal format (string to int) */
 	ram_address = simple_strtoul(argv[3], NULL, 16);
 
-	/* get the filesize in base 10 format */
-	file_size = simple_strtoul(argv[5], NULL, 10);
+	/* get the filesize in hexadecimal format */
+	file_size = simple_strtoul(argv[5], NULL, 16);
 
 	/* set the device as block device */
 	ext4fs_set_blk_dev(dev_desc, &info);
Signature · 1 line
1.8.3.1
#1 lore raw

PATCH 1/1 EXT4: Fix number base handling of "ext4write" command

Hi Wolfgang,

Quoted 5 lines

Unlike other commands (for example, "fatwrite"), ext4write would interpret the "sizebytes" as decimal number. This is not only inconsistend and unexpected to most users, it also breaks usage like this:

Quoted 7 lines
	tftp ${addr} ${name}
	ext4write mmc 0:2 ${addr} ${filename} ${filesize}

Change this to use the standard notation of base 16 input format. See also commit b770e88

WARNING: this is a change to the user interface!!

In other words you are breaking API :-) - but this change is more than welcome and you have got enough power to do it :-).

Quoted 14 lines
Cc Uma Shankar <[email protected]>
Cc Stephen Warren <[email protected]>
---
 common/cmd_ext4.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
common/cmd_ext4.c +0 -0
index 8289d25..68b047b 100644
@@ -79,8 +79,8 @@ int do_ext4_write(cmd_tbl_t *cmdtp, int flag, int

argc, /* get the address in hexadecimal format (string to int) */

common/cmd_ext4.c int do_ext4_write(cmd_tbl_t *cmdtp, int flag, int argc, · line 86

Quoted 9 lines
common/cmd_ext4.c +2 -2
 	ram_address = simple_strtoul(argv[3], NULL, 16);
 
-	/* get the filesize in base 10 format */
-	file_size = simple_strtoul(argv[5], NULL, 10);
+	/* get the filesize in hexadecimal format */
+	file_size = simple_strtoul(argv[5], NULL, 16);
 
 	/* set the device as block device */
 	ext4fs_set_blk_dev(dev_desc, &info);

My only comment is to add proper description to the ext4write commend description. Now it only says:

"<interface> <dev[:part]> <addr> <absolute filename path> [sizebytes]\n"

and I think, that we could come up with [sizebytes - HEX] or something similar.

Signature · 5 lines
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
#2 lore raw

PATCH 1/1 EXT4: Fix number base handling of "ext4write" command

Dear Lukasz,

Quoted 10 lines · In message <20140131102755.63297928@amdc2363> you wrote:
Quoted 7 lines
	ext4write mmc 0:2 ${addr} ${filename} ${filesize}

Change this to use the standard notation of base 16 input format. See also commit b770e88

WARNING: this is a change to the user interface!!

In other words you are breaking API :-) - but this change is more than welcome and you have got enough power to do it :-).

Yes, I'm breaking the current (incorrectly implemented) ABI to fix it and make it consistend with other use (for example, "fatwrite"). As is, it can only be used from the command line, but not from any scripts that refer for example to ${filesize}.

Quoted 7 lines

My only comment is to add proper description to the ext4write commend description. Now it only says:

"<interface> <dev[:part]> <addr> <absolute filename path> [sizebytes]\n"

and I think, that we could come up with [sizebytes - HEX] or something similar.

I do not see any such need. Hex input base is the established and documented default - ext4write is not a special command, so why should we mention this here when we do not mention it anywhere else?

Best regards,

Wolfgang Denk

Signature · 5 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
Advice is seldom welcome; and those who want it the most always like
it the least.                         -- Philip Earl of Chesterfield
#3 lore raw

PATCH 1/1 EXT4: Fix number base handling of "ext4write" command

Hi Wolfgang,

Quoted 11 lines

Dear Lukasz,

Quoted 8 lines · In message <20140131102755.63297928@amdc2363> you wrote:
Quoted 7 lines
	ext4write mmc 0:2 ${addr} ${filename} ${filesize}

Change this to use the standard notation of base 16 input format. See also commit b770e88

WARNING: this is a change to the user interface!!

Quoted 7 lines
Quoted 2 lines

In other words you are breaking API :-) - but this change is more than welcome and you have got enough power to do it :-).

Yes, I'm breaking the current (incorrectly implemented) ABI to fix it and make it consistend with other use (for example, "fatwrite"). As is, it can only be used from the command line, but not from any scripts that refer for example to ${filesize}.

And I'm totally with you with this change.

Quoted 7 lines
Quoted 7 lines

My only comment is to add proper description to the ext4write commend description. Now it only says:

"<interface> <dev[:part]> <addr> <absolute filename path> [sizebytes]\n"

Quoted 6 lines
Quoted 2 lines

and I think, that we could come up with [sizebytes - HEX] or something similar.

I do not see any such need. Hex input base is the established and documented default - ext4write is not a special command, so why should we mention this here when we do not mention it anywhere else?

If now all <fs>*write and <fs>*load commands accept only hex input, then I agree, that extra comment is not needed.

Quoted 5 lines

Best regards,

Wolfgang Denk

Signature · 5 lines
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
#4 lore raw

PATCH 1/1 EXT4: Fix number base handling of "ext4write" command

Dear Lukasz,

Quoted 7 lines · In message <20140131110818.07d79eac@amdc2363> you wrote:
Quoted 4 lines

I do not see any such need. Hex input base is the established and documented default - ext4write is not a special command, so why should we mention this here when we do not mention it anywhere else?

If now all <fs>*write and <fs>*load commands accept only hex input, then I agree, that extra comment is not needed.

Not only these, but all other commands - with the inglorious exception of the "sleep" command (but hey - there must be a bad example somewhere, right? ;-)

Best regards,

Wolfgang Denk

Signature · 7 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
There are two ways of constructing a software design. One way  is  to
make  it  so  simple that there are obviously no deficiencies and the
other is to make it so complicated that there are  no  obvious  defi-
ciencies.                             - Charles Anthony Richard Hoare
#5 lore raw
In reply to Wolfgang Denk
Quoted 8 lines · Wolfgang Denk

Unlike other commands (for example, "fatwrite"), ext4write would interpret the "sizebytes" as decimal number. This is not only inconsistend and unexpected to most users, it also breaks usage like this:

	tftp ${addr} ${name}
	ext4write mmc 0:2 ${addr} ${filename} ${filesize}
Quoted 8 lines

Change this to use the standard notation of base 16 input format. See also commit b770e88

WARNING: this is a change to the user interface!!

Cc Uma Shankar <[email protected]>
Cc Stephen Warren <[email protected]>

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140219/c7f8984c/attachment.pgp>