Re: [U-Boot] [PATCH] catch wrong load address passed to fatload / ext2load

2014-07-09 Thread Pavel Machek
On Thu 2014-07-10 01:08:28, Wolfgang Denk wrote:
> Dear Pavel Machek,
> 
> In message <20140709204257.ga28...@amd.pavel.ucw.cz> you wrote:
> > 
> > If filename is passed instead of address to ext2load or fatload,
> > u-boot silently accepts that, and uses 0 for load address and default
> > filename from environment. That is confusing, display help instead.
> > 
> > Signed-off-by: Pavel Machek 
> > 
> > diff --git a/fs/fs.c b/fs/fs.c
> > index 79d432d..ea15c5f 100644
> > --- a/fs/fs.c
> > +++ b/fs/fs.c
> > @@ -276,6 +276,7 @@ int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char 
> > * const argv[],
> > unsigned long pos;
> > int len_read;
> > unsigned long time;
> > +   char *ep;
> >  
> > if (argc < 2)
> > return CMD_RET_USAGE;
> > @@ -286,7 +287,9 @@ int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char 
> > * const argv[],
> > return 1;
> >  
> > if (argc >= 4) {
> > -   addr = simple_strtoul(argv[3], NULL, 16);
> > +   addr = simple_strtoul(argv[3], &ep, 16);
> > +   if (ep == argv[3] || *ep != '\0')
> > +   return CMD_RET_USAGE;
> 
> What happens in case of filenames that look like numbers, say "0"?
> It may be silly to use such, but it should not be impossible to do
> so.

It should be ok.

This catches error when someone passes non-number as a load
address. As long as you pass load address, nothing changes.

IOW  

fatload mmc 0:1 0 filename
# always worked/keeps working
fatload mmc 0:1 filename
# before my patch, this would try to parse filename as number, fail,
# and use default load address adn default filename. Not what user
# wanted.
fatload mmc 0:1 0 0
# Load filename "0" at address 0. Worked before, will work now.

Hope this helps,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] catch wrong load address passed to fatload / ext2load

2014-07-09 Thread Wolfgang Denk
Dear Pavel Machek,

In message <20140709204257.ga28...@amd.pavel.ucw.cz> you wrote:
> 
> If filename is passed instead of address to ext2load or fatload,
> u-boot silently accepts that, and uses 0 for load address and default
> filename from environment. That is confusing, display help instead.
> 
> Signed-off-by: Pavel Machek 
> 
> diff --git a/fs/fs.c b/fs/fs.c
> index 79d432d..ea15c5f 100644
> --- a/fs/fs.c
> +++ b/fs/fs.c
> @@ -276,6 +276,7 @@ int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * 
> const argv[],
>   unsigned long pos;
>   int len_read;
>   unsigned long time;
> + char *ep;
>  
>   if (argc < 2)
>   return CMD_RET_USAGE;
> @@ -286,7 +287,9 @@ int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * 
> const argv[],
>   return 1;
>  
>   if (argc >= 4) {
> - addr = simple_strtoul(argv[3], NULL, 16);
> + addr = simple_strtoul(argv[3], &ep, 16);
> + if (ep == argv[3] || *ep != '\0')
> + return CMD_RET_USAGE;

What happens in case of filenames that look like numbers, say "0"?
It may be silly to use such, but it should not be impossible to do so.

Viele Grüße,

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: w...@denx.de
One difference between a man and a machine is that a machine is quiet
when well oiled.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] catch wrong load address passed to fatload / ext2load

2014-07-09 Thread Pavel Machek
On Wed 2014-07-09 23:07:56, Marek Vasut wrote:
> On Wednesday, July 09, 2014 at 10:42:57 PM, Pavel Machek wrote:
> > If filename is passed instead of address to ext2load or fatload,
> > u-boot silently accepts that, and uses 0 for load address and default
> > filename from environment. That is confusing, display help instead.
> > 
> > Signed-off-by: Pavel Machek 
> 
> The handling of simple_strtoul() in fs.c could use improvement like this in 
> general. Do you mind expanding this patch ?

I'd prefer not to do that at this moment: I don't have working-enough
u-boot and hate to do changes without testing.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] catch wrong load address passed to fatload / ext2load

2014-07-09 Thread Marek Vasut
On Wednesday, July 09, 2014 at 10:42:57 PM, Pavel Machek wrote:
> If filename is passed instead of address to ext2load or fatload,
> u-boot silently accepts that, and uses 0 for load address and default
> filename from environment. That is confusing, display help instead.
> 
> Signed-off-by: Pavel Machek 

The handling of simple_strtoul() in fs.c could use improvement like this in 
general. Do you mind expanding this patch ?

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] catch wrong load address passed to fatload / ext2load

2014-07-09 Thread Pavel Machek
Hi!

> If filename is passed instead of address to ext2load or fatload,
> u-boot silently accepts that, and uses 0 for load address and default
> filename from environment. That is confusing, display help instead.
> 
> Signed-off-by: Pavel Machek 

Oops and actually I should warn... this patch is untested. I'm
currently fighting hard to get recent u-boot to run on socfpga.

Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH] catch wrong load address passed to fatload / ext2load

2014-07-09 Thread Pavel Machek

If filename is passed instead of address to ext2load or fatload,
u-boot silently accepts that, and uses 0 for load address and default
filename from environment. That is confusing, display help instead.

Signed-off-by: Pavel Machek 

diff --git a/fs/fs.c b/fs/fs.c
index 79d432d..ea15c5f 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -276,6 +276,7 @@ int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * 
const argv[],
unsigned long pos;
int len_read;
unsigned long time;
+   char *ep;
 
if (argc < 2)
return CMD_RET_USAGE;
@@ -286,7 +287,9 @@ int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * 
const argv[],
return 1;
 
if (argc >= 4) {
-   addr = simple_strtoul(argv[3], NULL, 16);
+   addr = simple_strtoul(argv[3], &ep, 16);
+   if (ep == argv[3] || *ep != '\0')
+   return CMD_RET_USAGE;
} else {
addr_str = getenv("loadaddr");
if (addr_str != NULL)

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot