[U-Boot] [PATCH] mkimage: fix argument parsing on BSD systems

The getopt(3) optstring '-' is a GNU extension which is not available on BSD systems like OS X.
Remove this dependency by implementing argument parsing in another way. This will also change the lately introduced '-b' switch behaviour.
Signed-off-by: Andreas Bießmann andreas.devel@googlemail.com ---
Makefile | 2 +- doc/mkimage.1 | 6 +++--- tools/mkimage.c | 33 ++++++++++++--------------------- 3 files changed, 16 insertions(+), 25 deletions(-)
diff --git a/Makefile b/Makefile index f03fec1..293fad0 100644 --- a/Makefile +++ b/Makefile @@ -898,7 +898,7 @@ ifdef CONFIG_SPL_LOAD_FIT MKIMAGEFLAGS_u-boot.img = -f auto -A $(ARCH) -T firmware -C none -O u-boot \ -a $(CONFIG_SYS_TEXT_BASE) -e $(CONFIG_SYS_UBOOT_START) \ -n "U-Boot $(UBOOTRELEASE) for $(BOARD) board" -E \ - -b $(patsubst %,arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST))) + $(patsubst %,-b arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST))) else MKIMAGEFLAGS_u-boot.img = -A $(ARCH) -T firmware -C none -O u-boot \ -a $(CONFIG_SYS_TEXT_BASE) -e $(CONFIG_SYS_UBOOT_START) \ diff --git a/doc/mkimage.1 b/doc/mkimage.1 index e0f210a..4b3a255 100644 --- a/doc/mkimage.1 +++ b/doc/mkimage.1 @@ -97,8 +97,8 @@ Set XIP (execute in place) flag. .B Create FIT image:
.TP -.BI "-b -Specifies that the following arguments are device tree binary files (.dtb). +.BI "-b [" "device tree file" "] +Appends the device tree binary file (.dtb) to the FIT.
.TP .BI "-c [" "comment" "]" @@ -211,7 +211,7 @@ automatic mode. No .its file is required. .B mkimage -f auto -A arm -O linux -T kernel -C none -a 43e00000 -e 0 \\ .br .B -c """Kernel 4.4 image for production devices""" -d vmlinuz \\ -.B -b /path/to/rk3288-firefly.dtb /path/to/rk3288-jerry.dtb kernel.itb +.B -b /path/to/rk3288-firefly.dtb -b /path/to/rk3288-jerry.dtb kernel.itb .fi
.SH HOMEPAGE diff --git a/tools/mkimage.c b/tools/mkimage.c index 2931783..b407aed 100644 --- a/tools/mkimage.c +++ b/tools/mkimage.c @@ -85,8 +85,8 @@ static void usage(const char *msg) " -x ==> set XIP (execute in place)\n", params.cmdname); fprintf(stderr, - " %s [-D dtc_options] [-f fit-image.its|-f auto|-F] [-b <dtb_list>] fit-image\n" - " <dtb_list> is used with -f auto, and is a space-separated list of .dtb files\n", + " %s [-D dtc_options] [-f fit-image.its|-f auto|-F] [-b <dtb> [-b <dtb>]] fit-image\n" + " <dtb> file is used with -f auto, it may occour multiple times.\n", params.cmdname); fprintf(stderr, " -D => set all options for device tree compiler\n" @@ -138,7 +138,7 @@ static void process_args(int argc, char **argv)
expecting = IH_TYPE_COUNT; /* Unknown */ while ((opt = getopt(argc, argv, - "-a:A:bcC:d:D:e:Ef:Fk:K:ln:O:rR:sT:vVx")) != -1) { + "a:A:b:cC:d:D:e:Ef:Fk:K:ln:O:rR:sT:vVx")) != -1) { switch (opt) { case 'a': params.addr = strtoull(optarg, &ptr, 16); @@ -155,6 +155,12 @@ static void process_args(int argc, char **argv) break; case 'b': expecting = IH_TYPE_FLATDT; + if (add_content(expecting, optarg)) { + fprintf(stderr, + "%s: Out of memory adding content '%s'", + params.cmdname, optarg); + exit(EXIT_FAILURE); + } break; case 'c': params.comment = optarg; @@ -243,29 +249,14 @@ static void process_args(int argc, char **argv) case 'x': params.xflag++; break; - case 1: - if (expecting == type || optind == argc) { - params.imagefile = optarg; - expecting = IH_TYPE_INVALID; - } else if (expecting == IH_TYPE_INVALID) { - fprintf(stderr, - "%s: Unknown content type: use -b before device tree files", - params.cmdname); - exit(EXIT_FAILURE); - } else { - if (add_content(expecting, optarg)) { - fprintf(stderr, - "%s: Out of memory adding content '%s'", - params.cmdname, optarg); - exit(EXIT_FAILURE); - } - } - break; default: usage("Invalid option"); } }
+ if (optind < argc && expecting == type) + params.imagefile = argv[optind]; + /* * For auto-generated FIT images we need to know the image type to put * in the FIT, which is separate from the file's image type (which

Hi Andreas,
On 30 April 2016 at 19:01, Andreas Bießmann andreas.devel@googlemail.com wrote:
The getopt(3) optstring '-' is a GNU extension which is not available on BSD systems like OS X.
Remove this dependency by implementing argument parsing in another way. This will also change the lately introduced '-b' switch behaviour.
Signed-off-by: Andreas Bießmann andreas.devel@googlemail.com
Makefile | 2 +- doc/mkimage.1 | 6 +++--- tools/mkimage.c | 33 ++++++++++++--------------------- 3 files changed, 16 insertions(+), 25 deletions(-)
This looks good to me and the code is cleaner but I'm a bit worried about the -b flag change. It makes it impossible to do something like
mkimage ... -b *.dtb output.fit
Perhaps this is a small price to pay?
Reviewed-by: Simon Glass sjg@chromium.org
Regards, Simon

On Sun, May 01, 2016 at 03:01:27AM +0200, Andreas Bießmann wrote:
The getopt(3) optstring '-' is a GNU extension which is not available on BSD systems like OS X.
Remove this dependency by implementing argument parsing in another way. This will also change the lately introduced '-b' switch behaviour.
Signed-off-by: Andreas Bießmann andreas.devel@googlemail.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

Hi Tom,
2016-05-03 1:32 GMT+02:00 Tom Rini trini@konsulko.com:
On Sun, May 01, 2016 at 03:01:27AM +0200, Andreas Bießmann wrote:
The getopt(3) optstring '-' is a GNU extension which is not available on
BSD
systems like OS X.
Remove this dependency by implementing argument parsing in another way.
This
will also change the lately introduced '-b' switch behaviour.
Signed-off-by: Andreas Bießmann andreas.devel@googlemail.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
This is breaking my mkimage for CONFIG_SPL_LOAD_FIT=y CONFIG_OF_LIST="zynq_zc702 zynq_zc706" (for zynq you have to apply this [PATCH] ARM: zynq: Add support for SPL_LOAD_FIT)
expecting value doesn't match type in this condition. if (optind < argc && expecting == type) params.imagefile = argv[optind];
Thanks, Michal
./tools/mkimage -f auto -A arm -T firmware -C none -O u-boot -a 0x4000000 -e 0x4000000 -n "U-Boot 2016.05-rc3-00060-g5eb59224edcb-dirty for zynq board" -E -b arch/arm/dts/zynq_zc702.dtb -b arch/arm/dts/zynq_zc706.dtb -d u-boot-nodtb.bin u-boot.img Error: Missing output filename Usage: ./tools/mkimage -l image -l ==> list image header information ./tools/mkimage [-x] -A arch -O os -T type -C comp -a addr -e ep -n name -d data_file[:data_file...] image -A ==> set architecture to 'arch' -O ==> set operating system to 'os' -T ==> set image type to 'type' -C ==> set compression type 'comp' -a ==> set load address to 'addr' (hex) -e ==> set entry point to 'ep' (hex) -n ==> set image name to 'name' -d ==> use image data from 'datafile' -x ==> set XIP (execute in place) ./tools/mkimage [-D dtc_options] [-f fit-image.its|-f auto|-F] [-b <dtb> [-b <dtb>]] fit-image <dtb> file is used with -f auto, it may occour multiple times. -D => set all options for device tree compiler -f => input filename for FIT source Signing / verified boot options: [-k keydir] [-K dtb] [ -c <comment>] [-r] -k => set directory containing private keys -K => write public keys to this .dtb file -c => add comment in signature node -F => re-sign existing FIT image -r => mark keys used as 'required' in dtb ./tools/mkimage -V ==> print version information and exit Use -T to see a list of available image types make: *** [u-boot.img] Error 1
Thanks, Michal --- Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/ Maintainer of Linux kernel - Xilinx Zynq ARM architecture Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform

Commit 7a439cadcf3192eb012a2432ca34670b676c74d2 broke generation of SPL loadable FIT images (CONFIG_SPL_LOAD_FIT). Fix it by removing the unnecessary storage of expected image type. This was a left over of the previous implementation. It is not longer necessary since the mkimage -b switch always has one parameter.
Signed-off-by: Andreas Bießmann andreas@biessmann.org ---
tools/mkimage.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/tools/mkimage.c b/tools/mkimage.c index b407aed..93d1c16 100644 --- a/tools/mkimage.c +++ b/tools/mkimage.c @@ -133,10 +133,8 @@ static void process_args(int argc, char **argv) char *ptr; int type = IH_TYPE_INVALID; char *datafile = NULL; - int expecting; int opt;
- expecting = IH_TYPE_COUNT; /* Unknown */ while ((opt = getopt(argc, argv, "a:A:b:cC:d:D:e:Ef:Fk:K:ln:O:rR:sT:vVx")) != -1) { switch (opt) { @@ -154,8 +152,7 @@ static void process_args(int argc, char **argv) usage("Invalid architecture"); break; case 'b': - expecting = IH_TYPE_FLATDT; - if (add_content(expecting, optarg)) { + if (add_content(IH_TYPE_FLATDT, optarg)) { fprintf(stderr, "%s: Out of memory adding content '%s'", params.cmdname, optarg); @@ -238,7 +235,6 @@ static void process_args(int argc, char **argv) show_image_types(); usage("Invalid image type"); } - expecting = type; break; case 'v': params.vflag++; @@ -254,7 +250,8 @@ static void process_args(int argc, char **argv) } }
- if (optind < argc && expecting == type) + /* The last parameter is expected to be the imagefile */ + if (optind < argc) params.imagefile = argv[optind];
/*

On Tuesday 03 May 2016 06:47 PM, Andreas Bießmann wrote:
Commit 7a439cadcf3192eb012a2432ca34670b676c74d2 broke generation of SPL loadable FIT images (CONFIG_SPL_LOAD_FIT). Fix it by removing the unnecessary storage of expected image type. This was a left over of the previous implementation. It is not longer necessary since the mkimage -b switch always has one parameter.
Thanks for the Patch. This fixes the build with CONFIG_SPL_LOAD_FIT enabled.
Tested-by: Lokesh Vutla lokeshvutla@ti.com
Thanks and regards, Lokesh
Signed-off-by: Andreas Bießmann andreas@biessmann.org
tools/mkimage.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/tools/mkimage.c b/tools/mkimage.c index b407aed..93d1c16 100644 --- a/tools/mkimage.c +++ b/tools/mkimage.c @@ -133,10 +133,8 @@ static void process_args(int argc, char **argv) char *ptr; int type = IH_TYPE_INVALID; char *datafile = NULL;
int expecting; int opt;
expecting = IH_TYPE_COUNT; /* Unknown */ while ((opt = getopt(argc, argv, "a:A:b:cC:d:D:e:Ef:Fk:K:ln:O:rR:sT:vVx")) != -1) { switch (opt) {
@@ -154,8 +152,7 @@ static void process_args(int argc, char **argv) usage("Invalid architecture"); break; case 'b':
expecting = IH_TYPE_FLATDT;
if (add_content(expecting, optarg)) {
if (add_content(IH_TYPE_FLATDT, optarg)) { fprintf(stderr, "%s: Out of memory adding content '%s'", params.cmdname, optarg);
@@ -238,7 +235,6 @@ static void process_args(int argc, char **argv) show_image_types(); usage("Invalid image type"); }
case 'v': params.vflag++;expecting = type; break;
@@ -254,7 +250,8 @@ static void process_args(int argc, char **argv) } }
- if (optind < argc && expecting == type)
/* The last parameter is expected to be the imagefile */
if (optind < argc) params.imagefile = argv[optind];
/*

On Tue, May 03, 2016 at 03:17:03PM +0200, Andreas Bießmann wrote:
Commit 7a439cadcf3192eb012a2432ca34670b676c74d2 broke generation of SPL loadable FIT images (CONFIG_SPL_LOAD_FIT). Fix it by removing the unnecessary storage of expected image type. This was a left over of the previous implementation. It is not longer necessary since the mkimage -b switch always has one parameter.
Tested-by: Lokesh Vutla lokeshvutla@ti.com Signed-off-by: Andreas Bießmann andreas@biessmann.org
Thanks for the quick fix, applied to u-boot/master, thanks!
participants (6)
-
Andreas Bießmann
-
Andreas Bießmann
-
Lokesh Vutla
-
Michal Simek
-
Simon Glass
-
Tom Rini