diff mbox

[U-Boot] mkimage: fix display of image types list

Message ID e073a9aed1fd517e72c878df61ac2a0ca14357d5.1498673983.git.baruch@tkos.co.il
State Changes Requested
Delegated to: Tom Rini
Headers show

Commit Message

Baruch Siach June 28, 2017, 6:19 p.m. UTC
Since commit 5b9d44df2307f (mkimage: Display a better list of available image
types) mkimage usage text suggest to "use -T to see a list of available image
types". Unfortunately, commit 02221f29deb8 (mkimage: Convert to use getopt())
broke that feature, because getopt() fails when -T has no option argument.

Make the -T option argument optional to restore the original behaviour.

Cc: Simon Glass <sjg@chromium.org>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 tools/mkimage.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Tom Rini June 29, 2017, 2 p.m. UTC | #1
On Wed, Jun 28, 2017 at 09:19:43PM +0300, Baruch Siach wrote:

> Since commit 5b9d44df2307f (mkimage: Display a better list of available image
> types) mkimage usage text suggest to "use -T to see a list of available image
> types". Unfortunately, commit 02221f29deb8 (mkimage: Convert to use getopt())
> broke that feature, because getopt() fails when -T has no option argument.
> 
> Make the -T option argument optional to restore the original behaviour.
> 
> Cc: Simon Glass <sjg@chromium.org>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
>  tools/mkimage.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/mkimage.c b/tools/mkimage.c
> index d982bc5665f1..2c7801e8f836 100644
> --- a/tools/mkimage.c
> +++ b/tools/mkimage.c
> @@ -144,7 +144,7 @@ static void process_args(int argc, char **argv)
>  	int opt;
>  
>  	while ((opt = getopt(argc, argv,
> -			     "a:A:b:c:C:d:D:e:Ef:Fk:i:K:ln:N:p:O:rR:qsT:vVx")) != -1) {
> +			     "a:A:b:c:C:d:D:e:Ef:Fk:i:K:ln:N:p:O:rR:qsT::vVx")) != -1) {
>  		switch (opt) {
>  		case 'a':
>  			params.addr = strtoull(optarg, &ptr, 16);
> @@ -260,8 +260,9 @@ static void process_args(int argc, char **argv)
>  			params.skipcpy = 1;
>  			break;
>  		case 'T':
> -			type = genimg_get_type_id(optarg);
> -			if (type < 0) {
> +			if (optarg)
> +				type = genimg_get_type_id(optarg);
> +			if (optarg == NULL || type < 0) {
>  				show_valid_options(IH_TYPE);
>  				usage("Invalid image type");
>  			}

This breaks a large number of boards, for example try building
Lamobo_R1.  Thanks!
Baruch Siach June 29, 2017, 5:36 p.m. UTC | #2
Hi Tom,

On Thu, Jun 29, 2017 at 10:00:55AM -0400, Tom Rini wrote:
> On Wed, Jun 28, 2017 at 09:19:43PM +0300, Baruch Siach wrote:
> > Since commit 5b9d44df2307f (mkimage: Display a better list of available image
> > types) mkimage usage text suggest to "use -T to see a list of available image
> > types". Unfortunately, commit 02221f29deb8 (mkimage: Convert to use getopt())
> > broke that feature, because getopt() fails when -T has no option argument.
> > 
> > Make the -T option argument optional to restore the original behaviour.
> > 
> > Cc: Simon Glass <sjg@chromium.org>
> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> > ---
> >  tools/mkimage.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/mkimage.c b/tools/mkimage.c
> > index d982bc5665f1..2c7801e8f836 100644
> > --- a/tools/mkimage.c
> > +++ b/tools/mkimage.c
> > @@ -144,7 +144,7 @@ static void process_args(int argc, char **argv)
> >  	int opt;
> >  
> >  	while ((opt = getopt(argc, argv,
> > -			     "a:A:b:c:C:d:D:e:Ef:Fk:i:K:ln:N:p:O:rR:qsT:vVx")) != -1) {
> > +			     "a:A:b:c:C:d:D:e:Ef:Fk:i:K:ln:N:p:O:rR:qsT::vVx")) != -1) {
> >  		switch (opt) {
> >  		case 'a':
> >  			params.addr = strtoull(optarg, &ptr, 16);
> > @@ -260,8 +260,9 @@ static void process_args(int argc, char **argv)
> >  			params.skipcpy = 1;
> >  			break;
> >  		case 'T':
> > -			type = genimg_get_type_id(optarg);
> > -			if (type < 0) {
> > +			if (optarg)
> > +				type = genimg_get_type_id(optarg);
> > +			if (optarg == NULL || type < 0) {
> >  				show_valid_options(IH_TYPE);
> >  				usage("Invalid image type");
> >  			}
> 
> This breaks a large number of boards, for example try building
> Lamobo_R1.  Thanks!

Thanks for testing. The optional argument glibc extension to getopt() doesn't 
work as expected. I'll post different suggestion in v2 shortly.

baruch
diff mbox

Patch

diff --git a/tools/mkimage.c b/tools/mkimage.c
index d982bc5665f1..2c7801e8f836 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -144,7 +144,7 @@  static void process_args(int argc, char **argv)
 	int opt;
 
 	while ((opt = getopt(argc, argv,
-			     "a:A:b:c:C:d:D:e:Ef:Fk:i:K:ln:N:p:O:rR:qsT:vVx")) != -1) {
+			     "a:A:b:c:C:d:D:e:Ef:Fk:i:K:ln:N:p:O:rR:qsT::vVx")) != -1) {
 		switch (opt) {
 		case 'a':
 			params.addr = strtoull(optarg, &ptr, 16);
@@ -260,8 +260,9 @@  static void process_args(int argc, char **argv)
 			params.skipcpy = 1;
 			break;
 		case 'T':
-			type = genimg_get_type_id(optarg);
-			if (type < 0) {
+			if (optarg)
+				type = genimg_get_type_id(optarg);
+			if (optarg == NULL || type < 0) {
 				show_valid_options(IH_TYPE);
 				usage("Invalid image type");
 			}