* [U-Boot] [PATCH] EXT4: Fix number base handling of "ext4write" command
@ 2014-01-31 8:28 Wolfgang Denk
2014-01-31 9:27 ` Lukasz Majewski
2014-02-19 15:49 ` [U-Boot] " Tom Rini
0 siblings, 2 replies; 6+ messages in thread
From: Wolfgang Denk @ 2014-01-31 8:28 UTC (permalink / raw)
To: u-boot
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!!
Signed-off-by: Wolfgang Denk <wd@denx.de>
Cc: Uma Shankar <uma.shankar@samsung.com>
Cc: Stephen Warren <swarren@nvidia.com>
---
common/cmd_ext4.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/common/cmd_ext4.c b/common/cmd_ext4.c
index 8289d25..68b047b 100644
--- a/common/cmd_ext4.c
+++ b/common/cmd_ext4.c
@@ -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);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] EXT4: Fix number base handling of "ext4write" command
2014-01-31 8:28 [U-Boot] [PATCH] EXT4: Fix number base handling of "ext4write" command Wolfgang Denk
@ 2014-01-31 9:27 ` Lukasz Majewski
2014-01-31 9:45 ` Wolfgang Denk
2014-02-19 15:49 ` [U-Boot] " Tom Rini
1 sibling, 1 reply; 6+ messages in thread
From: Lukasz Majewski @ 2014-01-31 9:27 UTC (permalink / raw)
To: u-boot
Hi Wolfgang,
> 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!!
In other words you are breaking API :-) - but this change is more than
welcome and you have got enough power to do it :-).
>
> Signed-off-by: Wolfgang Denk <wd@denx.de>
> Cc: Uma Shankar <uma.shankar@samsung.com>
> Cc: Stephen Warren <swarren@nvidia.com>
> ---
> common/cmd_ext4.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/common/cmd_ext4.c b/common/cmd_ext4.c
> index 8289d25..68b047b 100644
> --- a/common/cmd_ext4.c
> +++ b/common/cmd_ext4.c
> @@ -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);
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.
--
Best regards,
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] EXT4: Fix number base handling of "ext4write" command
2014-01-31 9:27 ` Lukasz Majewski
@ 2014-01-31 9:45 ` Wolfgang Denk
2014-01-31 10:08 ` Lukasz Majewski
0 siblings, 1 reply; 6+ messages in thread
From: Wolfgang Denk @ 2014-01-31 9:45 UTC (permalink / raw)
To: u-boot
Dear Lukasz,
In message <20140131102755.63297928@amdc2363> you wrote:
>
> > 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}.
> 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
--
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] EXT4: Fix number base handling of "ext4write" command
2014-01-31 9:45 ` Wolfgang Denk
@ 2014-01-31 10:08 ` Lukasz Majewski
2014-01-31 11:26 ` Wolfgang Denk
0 siblings, 1 reply; 6+ messages in thread
From: Lukasz Majewski @ 2014-01-31 10:08 UTC (permalink / raw)
To: u-boot
Hi Wolfgang,
> Dear Lukasz,
>
> In message <20140131102755.63297928@amdc2363> you wrote:
> >
> > > 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}.
And I'm totally with you with this change.
>
> > 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?
If now all <fs>*write and <fs>*load commands accept only hex input,
then I agree, that extra comment is not needed.
>
> Best regards,
>
> Wolfgang Denk
>
--
Best regards,
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] EXT4: Fix number base handling of "ext4write" command
2014-01-31 10:08 ` Lukasz Majewski
@ 2014-01-31 11:26 ` Wolfgang Denk
0 siblings, 0 replies; 6+ messages in thread
From: Wolfgang Denk @ 2014-01-31 11:26 UTC (permalink / raw)
To: u-boot
Dear Lukasz,
In message <20140131110818.07d79eac@amdc2363> you wrote:
>
> > 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
--
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] EXT4: Fix number base handling of "ext4write" command
2014-01-31 8:28 [U-Boot] [PATCH] EXT4: Fix number base handling of "ext4write" command Wolfgang Denk
2014-01-31 9:27 ` Lukasz Majewski
@ 2014-02-19 15:49 ` Tom Rini
1 sibling, 0 replies; 6+ messages in thread
From: Tom Rini @ 2014-02-19 15:49 UTC (permalink / raw)
To: u-boot
On Fri, Jan 31, 2014 at 09:28:25AM +0100, Wolfgang Denk wrote:
> 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!!
>
> Signed-off-by: Wolfgang Denk <wd@denx.de>
> Cc: Uma Shankar <uma.shankar@samsung.com>
> Cc: Stephen Warren <swarren@nvidia.com>
Applied to u-boot/master, thanks!
--
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-02-19 15:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-31 8:28 [U-Boot] [PATCH] EXT4: Fix number base handling of "ext4write" command Wolfgang Denk
2014-01-31 9:27 ` Lukasz Majewski
2014-01-31 9:45 ` Wolfgang Denk
2014-01-31 10:08 ` Lukasz Majewski
2014-01-31 11:26 ` Wolfgang Denk
2014-02-19 15:49 ` [U-Boot] " Tom Rini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox