mkimage: Tidy up error handling
cover mkimage: Tidy up error handling
There are a few problems when mkimage is provided with invalid arguments. In one case it crashes. When an invalid image type it is provided it lists the valid types, but this is not implemented for compression, architecture or OS.
This series tidies this up a little, to make mkimage more friendly.
Simon Glass (14):
mkimage: Honour the default image type with auto-fit
mkimage: Explain the auto-fit imagefile special case
mkimage: Require a data file when auto-fit is used
mkimage: Drop premature setting of params.fit_image_type
mkimage: Drop blank line before main()
image: Correct auto-fit architecture property name
image: Convert the IH_... values to enums
image: Create a table of information for each category
image: Add a name for invalid types
image: Add functions to obtain category information
mkimage: Allow display of a list of any image header category
mkimage: Use generic code for showing an 'image type' error
mkimage: Show item lists for all categories
tools: Allow building with debug enabled
Kconfig | 9 +++ Makefile | 3 +- common/image.c | 87 ++++++++++++++++++++- include/image.h | 230 ++++++++++++++++++++++++++++++++++-------------------- tools/fit_image.c | 3 +- tools/mkimage.c | 69 +++++++++------- 6 files changed, 280 insertions(+), 121 deletions(-)
Signature · 1 line
2.8.0.rc3.226.g39d4020
PATCH 1/14 mkimage: Honour the default image type with auto-fit
The default image type is supposed to be IH_TYPE_KERNEL, as set in the 'params' variable. Honour this with auto-fit also.
---
tools/mkimage.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/mkimage.c +1 -1
@@ -272,7 +272,7 @@ static void process_args(int argc, char **argv)* will always be IH_TYPE_FLATDT in this case). */ if (params.type ==IH_TYPE_FLATDT ) { - params.fit_image_type = type; + params.fit_image_type = type ? type :IH_TYPE_KERNEL ;if (!params.auto_its) params.datafile = datafile; }else if (type !=IH_TYPE_INVALID ) {
Signature · 1 line
2.8.0.rc3.226.g39d4020
PATCH 1/14 mkimage: Honour the default image type with auto-fit
Quoted 4 lines · Simon Glass
The default image type is supposed to be
IH_TYPE_KERNEL, as set in the 'params' variable. Honour this with auto-fit also.Signed-off-by Simon Glass <[email protected]>
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/f16fbdfb/attachment.sig>
PATCH 1/14 mkimage: Honour the default image type with auto-fit
In reply to Simon GlassQuoted 5 lines · Simon Glass
The default image type is supposed to be
IH_TYPE_KERNEL, as set in the 'params' variable. Honour this with auto-fit also.Signed-off-by Simon Glass <[email protected]>Reviewed-by Tom Rini <[email protected]>
Applied to u-boot/master, thanks!
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160716/49a75526/attachment.sig>
PATCH 2/14 mkimage: Explain the auto-fit imagefile special case
In reply to Simon GlassThere is a special case in the code when auto-fit is used. Add a comment to make it easier to understand why this is needed.
---
tools/mkimage.c | 1 + 1 file changed, 1 insertion(+)
tools/mkimage.c +1 -0
@@ -273,6 +273,7 @@ static void process_args(int argc, char **argv)*/ if (params.type ==IH_TYPE_FLATDT ) { params.fit_image_type = type ? type :IH_TYPE_KERNEL ; +/* For auto_its, datafile is always 'auto' */ if (!params.auto_its) params.datafile = datafile; }else if (type !=IH_TYPE_INVALID ) {
Signature · 1 line
2.8.0.rc3.226.g39d4020
PATCH 2/14 mkimage: Explain the auto-fit imagefile special case
Quoted 4 lines · Simon Glass
There is a special case in the code when auto-fit is used. Add a comment to make it easier to understand why this is needed.
Signed-off-by Simon Glass <[email protected]>
PATCH 2/14 mkimage: Explain the auto-fit imagefile special case
In reply to Simon GlassQuoted 5 lines · Simon Glass
There is a special case in the code when auto-fit is used. Add a comment to make it easier to understand why this is needed.
Signed-off-by Simon Glass <[email protected]>Acked-by Joe Hershberger <[email protected]>
Applied to u-boot/master, thanks!
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160716/58b728ce/attachment.sig>
PATCH 3/14 mkimage: Require a data file when auto-fit is used
In reply to Simon GlassWhen auto-fit is used, it is not valid to create a FIT without an image file. Add a check for this to avoid a very confusing error message later ("Can't open (null): Bad address").
---
tools/mkimage.c | 2 ++ 1 file changed, 2 insertions(+)
tools/mkimage.c +2 -0
@@ -276,6 +276,8 @@ static void process_args(int argc, char **argv)/* For auto_its, datafile is always 'auto' */ if (!params.auto_its) params.datafile = datafile; +else if (!params.datafile) +usage ("Missing data file for auto-FIT (use -d)" ); }else if (type !=IH_TYPE_INVALID ) { params.type = type; }
Signature · 1 line
2.8.0.rc3.226.g39d4020
PATCH 3/14 mkimage: Require a data file when auto-fit is used
Quoted 5 lines · Simon Glass
When auto-fit is used, it is not valid to create a FIT without an image file. Add a check for this to avoid a very confusing error message later ("Can't open (null): Bad address").
Signed-off-by Simon Glass <[email protected]>
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/bd9a31e5/attachment.sig>
PATCH 3/14 mkimage: Require a data file when auto-fit is used
In reply to Simon GlassQuoted 6 lines · Simon Glass
When auto-fit is used, it is not valid to create a FIT without an image file. Add a check for this to avoid a very confusing error message later ("Can't open (null): Bad address").
Signed-off-by Simon Glass <[email protected]>Reviewed-by Tom Rini <[email protected]>
Applied to u-boot/master, thanks!
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160716/881f3935/attachment.sig>
PATCH 4/14 mkimage: Drop premature setting of params.fit_image_type
In reply to Simon GlassThere is no need to set params.fit_image_type while parsing the arguments. It is set up later anyway.
---
tools/mkimage.c | 1 - 1 file changed, 1 deletion(-)
tools/mkimage.c +0 -1
@@ -197,7 +197,6 @@ static void process_args(int argc, char **argv)* The flattened image tree (FIT) format * requires a flattened device tree image type */ - params.fit_image_type = params.type; params.type =IH_TYPE_FLATDT ; params.fflag =1 ;break ;
Signature · 1 line
2.8.0.rc3.226.g39d4020
PATCH 4/14 mkimage: Drop premature setting of params.fit_image_type
Quoted 4 lines · Simon Glass
There is no need to set params.fit_image_type while parsing the arguments. It is set up later anyway.
Signed-off-by Simon Glass <[email protected]>
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/08c49643/attachment.sig>
PATCH 4/14 mkimage: Drop premature setting of params.fit_image_type
In reply to Simon GlassQuoted 5 lines · Simon Glass
There is no need to set params.fit_image_type while parsing the arguments. It is set up later anyway.
Signed-off-by Simon Glass <[email protected]>Reviewed-by Tom Rini <[email protected]>
Applied to u-boot/master, thanks!
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160716/d4de3ba5/attachment.sig>
PATCH 5/14 mkimage: Drop blank line before main()
In reply to Simon GlassThis is not needed. Drop it.
---
tools/mkimage.c | 1 - 1 file changed, 1 deletion(-)
tools/mkimage.c +0 -1
@@ -285,7 +285,6 @@ static void process_args(int argc, char **argv)usage ("Missing output filename" ); } -int main (int argc,char **argv) {int ifd =-1 ;
Signature · 1 line
2.8.0.rc3.226.g39d4020
PATCH 5/14 mkimage: Drop blank line before main()
Quoted 3 lines · Simon Glass
This is not needed. Drop it.
Signed-off-by Simon Glass <[email protected]>
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/8b4ea716/attachment.sig>
PATCH 5/14 mkimage: Drop blank line before main()
In reply to Simon GlassQuoted 4 lines · Simon Glass
This is not needed. Drop it.
Signed-off-by Simon Glass <[email protected]>Reviewed-by Tom Rini <[email protected]>
Applied to u-boot/master, thanks!
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160716/684a7931/attachment.sig>
PATCH 6/14 image: Correct auto-fit architecture property name
In reply to Simon GlassThe fit_write_images() function incorrectly uses the long name for the architecture. This cannot be parsed with the FIT is read. Fix this by using the short name instead.
---
tools/fit_image.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
tools/fit_image.c +2 -1
@@ -195,7 +195,8 @@ static int fit_write_images(struct image_tool_params *params, char *fdt)fdt_begin_node (fdt, str);fdt_property_string (fdt,"description" , params->imagename);fdt_property_string (fdt,"type" , typename); -fdt_property_string (fdt,"arch" ,genimg_get_arch_name (params->arch)); +fdt_property_string (fdt,"arch" , +genimg_get_arch_short_name (params->arch));fdt_property_string (fdt,"os" ,genimg_get_os_short_name (params->os));fdt_property_string (fdt,"compression" ,genimg_get_comp_short_name (params->comp));
Signature · 1 line
2.8.0.rc3.226.g39d4020
PATCH 6/14 image: Correct auto-fit architecture property name
Quoted 5 lines · Simon Glass
The
fit_write_images()function incorrectly uses the long name for the architecture. This cannot be parsed with the FIT is read. Fix this by using the short name instead.Signed-off-by Simon Glass <[email protected]>
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/221b98ca/attachment.sig>
PATCH 6/14 image: Correct auto-fit architecture property name
In reply to Simon GlassQuoted 6 lines · Simon Glass
The
fit_write_images()function incorrectly uses the long name for the architecture. This cannot be parsed with the FIT is read. Fix this by using the short name instead.Signed-off-by Simon Glass <[email protected]>Reviewed-by Tom Rini <[email protected]>
Applied to u-boot/master, thanks!
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160716/fd55bc94/attachment.sig>
PATCH 7/14 image: Convert the IH_... values to enums
In reply to Simon GlassWe need to know the number of values of each category (architecture, compression, OS and image type). To make this value easier to maintain, convert all values to enums. The count is then automatic.
---
include/image.h | 187 ++++++++++++++++++++++++++++++-------------------------- 1 file changed, 101 insertions(+), 86 deletions(-)
include/image.h +101 -86
@@ -126,59 +126,67 @@ struct lmb;/* * Operating System Codes */ -#define IH_OS_INVALID 0/* Invalid OS */ -#define IH_OS_OPENBSD 1/* OpenBSD */ -#define IH_OS_NETBSD 2/* NetBSD */ -#define IH_OS_FREEBSD 3/* FreeBSD */ -#define IH_OS_4_4BSD 4/* 4.4BSD */ -#define IH_OS_LINUX 5/* Linux */ -#define IH_OS_SVR4 6/* SVR4 */ -#define IH_OS_ESIX 7/* Esix */ -#define IH_OS_SOLARIS 8/* Solaris */ -#define IH_OS_IRIX 9/* Irix */ -#define IH_OS_SCO 10/* SCO */ -#define IH_OS_DELL 11/* Dell */ -#define IH_OS_NCR 12/* NCR */ -#define IH_OS_LYNXOS 13/* LynxOS */ -#define IH_OS_VXWORKS 14/* VxWorks */ -#define IH_OS_PSOS 15/* pSOS */ -#define IH_OS_QNX 16/* QNX */ -#define IH_OS_U_BOOT 17/* Firmware */ -#define IH_OS_RTEMS 18/* RTEMS */ -#define IH_OS_ARTOS 19/* ARTOS */ -#define IH_OS_UNITY 20/* Unity OS */ -#define IH_OS_INTEGRITY 21/* INTEGRITY */ -#define IH_OS_OSE 22/* OSE */ -#define IH_OS_PLAN9 23/* Plan 9 */ -#define IH_OS_OPENRTOS 24/* OpenRTOS */ +enum { +IH_OS_INVALID =0 ,/* Invalid OS */ +IH_OS_OPENBSD ,/* OpenBSD */ +IH_OS_NETBSD ,/* NetBSD */ +IH_OS_FREEBSD ,/* FreeBSD */ +IH_OS_4_4BSD ,/* 4.4BSD */ +IH_OS_LINUX ,/* Linux */ +IH_OS_SVR4 ,/* SVR4 */ +IH_OS_ESIX ,/* Esix */ +IH_OS_SOLARIS ,/* Solaris */ +IH_OS_IRIX ,/* Irix */ +IH_OS_SCO ,/* SCO */ +IH_OS_DELL ,/* Dell */ +IH_OS_NCR ,/* NCR */ +IH_OS_LYNXOS ,/* LynxOS */ +IH_OS_VXWORKS ,/* VxWorks */ +IH_OS_PSOS ,/* pSOS */ +IH_OS_QNX ,/* QNX */ +IH_OS_U_BOOT ,/* Firmware */ +IH_OS_RTEMS ,/* RTEMS */ +IH_OS_ARTOS ,/* ARTOS */ +IH_OS_UNITY ,/* Unity OS */ +IH_OS_INTEGRITY ,/* INTEGRITY */ +IH_OS_OSE ,/* OSE */ +IH_OS_PLAN9 ,/* Plan 9 */ +IH_OS_OPENRTOS ,/* OpenRTOS */ + +IH_OS_COUNT , +};/* * CPU Architecture Codes (supported by Linux) */ -#define IH_ARCH_INVALID 0/* Invalid CPU */ -#define IH_ARCH_ALPHA 1/* Alpha */ -#define IH_ARCH_ARM 2/* ARM */ -#define IH_ARCH_I386 3/* Intel x86 */ -#define IH_ARCH_IA64 4/* IA64 */ -#define IH_ARCH_MIPS 5/* MIPS */ -#define IH_ARCH_MIPS64 6/* MIPS 64 Bit */ -#define IH_ARCH_PPC 7/* PowerPC */ -#define IH_ARCH_S390 8/* IBM S390 */ -#define IH_ARCH_SH 9/* SuperH */ -#define IH_ARCH_SPARC 10/* Sparc */ -#define IH_ARCH_SPARC64 11/* Sparc 64 Bit */ -#define IH_ARCH_M68K 12/* M68K */ -#define IH_ARCH_MICROBLAZE 14/* MicroBlaze */ -#define IH_ARCH_NIOS2 15/* Nios-II */ -#define IH_ARCH_BLACKFIN 16/* Blackfin */ -#define IH_ARCH_AVR32 17/* AVR32 */ -#define IH_ARCH_ST200 18/* STMicroelectronics ST200 */ -#define IH_ARCH_SANDBOX 19/* Sandbox architecture (test only) */ -#define IH_ARCH_NDS32 20/* ANDES Technology - NDS32 */ -#define IH_ARCH_OPENRISC 21/* OpenRISC 1000 */ -#define IH_ARCH_ARM64 22/* ARM64 */ -#define IH_ARCH_ARC 23/* Synopsys DesignWare ARC */ -#define IH_ARCH_X86_64 24/* AMD x86_64, Intel and Via */ +enum { +IH_ARCH_INVALID =0 ,/* Invalid CPU */ +IH_ARCH_ALPHA ,/* Alpha */ +IH_ARCH_ARM ,/* ARM */ +IH_ARCH_I386 ,/* Intel x86 */ +IH_ARCH_IA64 ,/* IA64 */ +IH_ARCH_MIPS ,/* MIPS */ +IH_ARCH_MIPS64 ,/* MIPS 64 Bit */ +IH_ARCH_PPC ,/* PowerPC */ +IH_ARCH_S390 ,/* IBM S390 */ +IH_ARCH_SH ,/* SuperH */ +IH_ARCH_SPARC ,/* Sparc */ +IH_ARCH_SPARC64 ,/* Sparc 64 Bit */ +IH_ARCH_M68K ,/* M68K */ +IH_ARCH_MICROBLAZE ,/* MicroBlaze */ +IH_ARCH_NIOS2 ,/* Nios-II */ +IH_ARCH_BLACKFIN ,/* Blackfin */ +IH_ARCH_AVR32 ,/* AVR32 */ +IH_ARCH_ST200 ,/* STMicroelectronics ST200 */ +IH_ARCH_SANDBOX ,/* Sandbox architecture (test only) */ +IH_ARCH_NDS32 ,/* ANDES Technology - NDS32 */ +IH_ARCH_OPENRISC ,/* OpenRISC 1000 */ +IH_ARCH_ARM64 ,/* ARM64 */ +IH_ARCH_ARC ,/* Synopsys DesignWare ARC */ +IH_ARCH_X86_64 ,/* AMD x86_64, Intel and Via */ + +IH_ARCH_COUNT , +};/* * Image Types
@@ -219,47 +227,54 @@ struct lmb;* as command interpreter (=> Shell Scripts). */ -#define IH_TYPE_INVALID 0/* Invalid Image */ -#define IH_TYPE_STANDALONE 1/* Standalone Program */ -#define IH_TYPE_KERNEL 2/* OS Kernel Image */ -#define IH_TYPE_RAMDISK 3/* RAMDisk Image */ -#define IH_TYPE_MULTI 4/* Multi-File Image */ -#define IH_TYPE_FIRMWARE 5/* Firmware Image */ -#define IH_TYPE_SCRIPT 6/* Script file */ -#define IH_TYPE_FILESYSTEM 7/* Filesystem Image (any type) */ -#define IH_TYPE_FLATDT 8/* Binary Flat Device Tree Blob */ -#define IH_TYPE_KWBIMAGE 9/* Kirkwood Boot Image */ -#define IH_TYPE_IMXIMAGE 10/* Freescale IMXBoot Image */ -#define IH_TYPE_UBLIMAGE 11/* Davinci UBL Image */ -#define IH_TYPE_OMAPIMAGE 12/* TI OMAP Config Header Image */ -#define IH_TYPE_AISIMAGE 13/* TI Davinci AIS Image */ -#define IH_TYPE_KERNEL_NOLOAD 14/* OS Kernel Image, can run from any load address */ -#define IH_TYPE_PBLIMAGE 15/* Freescale PBL Boot Image */ -#define IH_TYPE_MXSIMAGE 16/* Freescale MXSBoot Image */ -#define IH_TYPE_GPIMAGE 17/* TI Keystone GPHeader Image */ -#define IH_TYPE_ATMELIMAGE 18/* ATMEL ROM bootable Image */ -#define IH_TYPE_SOCFPGAIMAGE 19/* Altera SOCFPGA Preloader */ -#define IH_TYPE_X86_SETUP 20/* x86 setup.bin Image */ -#define IH_TYPE_LPC32XXIMAGE 21/* x86 setup.bin Image */ -#define IH_TYPE_LOADABLE 22/* A list of typeless images */ -#define IH_TYPE_RKIMAGE 23/* Rockchip Boot Image */ -#define IH_TYPE_RKSD 24/* Rockchip SD card */ -#define IH_TYPE_RKSPI 25/* Rockchip SPI image */ -#define IH_TYPE_ZYNQIMAGE 26/* Xilinx Zynq Boot Image */ -#define IH_TYPE_ZYNQMPIMAGE 27/* Xilinx ZynqMP Boot Image */ -#define IH_TYPE_FPGA 28/* FPGA Image */ - -#define IH_TYPE_COUNT 29/* Number of image types */ +enum { +IH_TYPE_INVALID =0 ,/* Invalid Image */ +IH_TYPE_STANDALONE ,/* Standalone Program */ +IH_TYPE_KERNEL ,/* OS Kernel Image */ +IH_TYPE_RAMDISK ,/* RAMDisk Image */ +IH_TYPE_MULTI ,/* Multi-File Image */ +IH_TYPE_FIRMWARE ,/* Firmware Image */ +IH_TYPE_SCRIPT ,/* Script file */ +IH_TYPE_FILESYSTEM ,/* Filesystem Image (any type) */ +IH_TYPE_FLATDT ,/* Binary Flat Device Tree Blob */ +IH_TYPE_KWBIMAGE ,/* Kirkwood Boot Image */ +IH_TYPE_IMXIMAGE ,/* Freescale IMXBoot Image */ +IH_TYPE_UBLIMAGE ,/* Davinci UBL Image */ +IH_TYPE_OMAPIMAGE ,/* TI OMAP Config Header Image */ +IH_TYPE_AISIMAGE ,/* TI Davinci AIS Image */ +/* OS Kernel Image, can run from any load address */ +IH_TYPE_KERNEL_NOLOAD , +IH_TYPE_PBLIMAGE ,/* Freescale PBL Boot Image */ +IH_TYPE_MXSIMAGE ,/* Freescale MXSBoot Image */ +IH_TYPE_GPIMAGE ,/* TI Keystone GPHeader Image */ +IH_TYPE_ATMELIMAGE ,/* ATMEL ROM bootable Image */ +IH_TYPE_SOCFPGAIMAGE ,/* Altera SOCFPGA Preloader */ +IH_TYPE_X86_SETUP ,/* x86 setup.bin Image */ +IH_TYPE_LPC32XXIMAGE ,/* x86 setup.bin Image */ +IH_TYPE_LOADABLE ,/* A list of typeless images */ +IH_TYPE_RKIMAGE ,/* Rockchip Boot Image */ +IH_TYPE_RKSD ,/* Rockchip SD card */ +IH_TYPE_RKSPI ,/* Rockchip SPI image */ +IH_TYPE_ZYNQIMAGE ,/* Xilinx Zynq Boot Image */ +IH_TYPE_ZYNQMPIMAGE ,/* Xilinx ZynqMP Boot Image */ +IH_TYPE_FPGA ,/* FPGA Image */ + +IH_TYPE_COUNT ,/* Number of image types */ +};/* * Compression Types */ -#define IH_COMP_NONE 0/* No Compression Used */ -#define IH_COMP_GZIP 1/* gzip Compression Used */ -#define IH_COMP_BZIP2 2/* bzip2 Compression Used */ -#define IH_COMP_LZMA 3/* lzma Compression Used */ -#define IH_COMP_LZO 4/* lzo Compression Used */ -#define IH_COMP_LZ4 5/* lz4 Compression Used */ +enum { +IH_COMP_NONE =0 ,/* No Compression Used */ +IH_COMP_GZIP ,/* gzip Compression Used */ +IH_COMP_BZIP2 ,/* bzip2 Compression Used */ +IH_COMP_LZMA ,/* lzma Compression Used */ +IH_COMP_LZO ,/* lzo Compression Used */ +IH_COMP_LZ4 ,/* lz4 Compression Used */ + +IH_COMP_COUNT , +};#define IH_MAGIC 0x27051956/* Image Magic Number */ #define IH_NMLEN 32/* Image Name Length */
Signature · 1 line
2.8.0.rc3.226.g39d4020
PATCH 7/14 image: Convert the IH_... values to enums
Quoted 5 lines · Simon Glass
We need to know the number of values of each category (architecture, compression, OS and image type). To make this value easier to maintain, convert all values to enums. The count is then automatic.
Signed-off-by Simon Glass <[email protected]>
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/768205ee/attachment.sig>
PATCH 7/14 image: Convert the IH_... values to enums
In reply to Simon GlassQuoted 6 lines · Simon Glass
We need to know the number of values of each category (architecture, compression, OS and image type). To make this value easier to maintain, convert all values to enums. The count is then automatic.
Signed-off-by Simon Glass <[email protected]>Reviewed-by Tom Rini <[email protected]>
Applied to u-boot/master, thanks!
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160716/d51ac4d6/attachment.sig>
PATCH 8/14 image: Create a table of information for each category
In reply to Simon GlassAdd a table that contains the category name, the number of items in each category and a pointer to the table of items. This will allow us to use generic code to deal with the categories.
---
common/image.c | 13 +++++++++++++ include/image.h | 9 +++++++++ 2 files changed, 22 insertions(+)
common/image.c +13 -0
@@ -176,6 +176,19 @@ static const table_entry_t uimage_comp[] = { { -1, "", "", }, }; +struct table_info { +const char *desc; +int count; +const table_entry_t *table; +}; + +static const struct table_info table_info[IH_COUNT ] = { + {"architecture" ,IH_ARCH_COUNT , uimage_arch }, + {"compression" ,IH_COMP_COUNT , uimage_comp }, + {"operating system" ,IH_OS_COUNT , uimage_os }, + {"image type" ,IH_TYPE_COUNT , uimage_type }, +}; +/*****************************************************************************/ /* Legacy format routines */ /*****************************************************************************/
include/image.h +9 -0
@@ -123,6 +123,15 @@ struct lmb;# define IMAGE_OF_SYSTEM_SETUP 0#endif +enum ih_category { +IH_ARCH , +IH_COMP , +IH_OS , +IH_TYPE , + +IH_COUNT , +}; +/* * Operating System Codes */
Signature · 1 line
2.8.0.rc3.226.g39d4020
PATCH 8/14 image: Create a table of information for each category
Quoted 5 lines · Simon Glass
Add a table that contains the category name, the number of items in each category and a pointer to the table of items. This will allow us to use generic code to deal with the categories.
Signed-off-by Simon Glass <[email protected]>
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/175cc847/attachment.sig>
PATCH 8/14 image: Create a table of information for each category
In reply to Simon GlassQuoted 6 lines · Simon Glass
Add a table that contains the category name, the number of items in each category and a pointer to the table of items. This will allow us to use generic code to deal with the categories.
Signed-off-by Simon Glass <[email protected]>Reviewed-by Tom Rini <[email protected]>
Applied to u-boot/master, thanks!
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160716/e3f44ff5/attachment.sig>
PATCH 9/14 image: Add a name for invalid types
In reply to Simon GlassAt present the name is NULL, which prevents qsort() fromp being used. Use the name "invalid" instead.
---
common/image.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
common/image.c +3 -3
@@ -69,7 +69,7 @@ static const image_header_t *image_get_ramdisk(ulong rd_addr, uint8_t arch,#endif static const table_entry_t uimage_arch[] = { - {IH_ARCH_INVALID ,NULL ,"Invalid ARCH" , }, + {IH_ARCH_INVALID ,"invalid" ,"Invalid ARCH" , }, {IH_ARCH_ALPHA ,"alpha" ,"Alpha" , }, {IH_ARCH_ARM ,"arm" ,"ARM" , }, {IH_ARCH_I386 ,"x86" ,"Intel x86" , },
@@ -97,7 +97,7 @@ static const table_entry_t uimage_arch[] = { };static const table_entry_t uimage_os[] = { - {IH_OS_INVALID ,NULL ,"Invalid OS" , }, + {IH_OS_INVALID ,"invalid" ,"Invalid OS" , }, {IH_OS_LINUX ,"linux" ,"Linux" , }, #if defined(CONFIG_LYNXKDI) || defined(USE_HOSTCC) { IH_OS_LYNXOS, "lynxos", "LynxOS", },
@@ -144,7 +144,7 @@ static const table_entry_t uimage_type[] = { { IH_TYPE_KERNEL_NOLOAD, "kernel_noload", "Kernel Image (no loading done)", }, { IH_TYPE_KWBIMAGE, "kwbimage", "Kirkwood Boot Image",}, { IH_TYPE_IMXIMAGE, "imximage", "Freescale i.MX Boot Image",}, - { IH_TYPE_INVALID, NULL, "Invalid Image", }, + { IH_TYPE_INVALID, "invalid", "Invalid Image", }, { IH_TYPE_MULTI, "multi", "Multi-File Image", }, { IH_TYPE_OMAPIMAGE, "omapimage", "TI OMAP SPL With GP CH",}, { IH_TYPE_PBLIMAGE, "pblimage", "Freescale PBL Boot Image",},
Signature · 1 line
2.8.0.rc3.226.g39d4020
PATCH 9/14 image: Add a name for invalid types
Quoted 4 lines · Simon Glass
At present the name is NULL, which prevents
qsort()fromp being used. Use the name "invalid" instead.Signed-off-by Simon Glass <[email protected]>
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/8b88591c/attachment.sig>
PATCH 9/14 image: Add a name for invalid types
In reply to Simon GlassQuoted 5 lines · Simon Glass
At present the name is NULL, which prevents
qsort()fromp being used. Use the name "invalid" instead.Signed-off-by Simon Glass <[email protected]>Reviewed-by Tom Rini <[email protected]>
Applied to u-boot/master, thanks!
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160716/d5677d93/attachment.sig>
PATCH 10/14 image: Add functions to obtain category information
In reply to Simon GlassAdd generic functions which can look up information about a category:
- the number of items in the category
- the category description
- an item long time
- an item short time
---
common/image.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ include/image.h | 34 +++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+)
common/image.c +68 -0
@@ -583,6 +583,74 @@ const table_entry_t *get_table_entry(const table_entry_t *table, int id)return NULL ; } +static const char *unknown_msg (enum ih_category category) +{ +static char msg[30 ]; + +strcpy (msg,"Unknown " ); +strcat (msg, table_info[category].desc); + +return msg; +} + +/** +* get_cat_table_entry_name - translate entry id to long name +* @category: category to look up (enum ih_category) +* @id: entry id to be translated +* +* This will scan the translation table trying to find the entry that matches +* the given id. +* +* @retur long entry name if translation succeeds; error string on failure +*/ +const char *genimg_get_cat_name (enum ih_category category,uint id) +{ +const table_entry_t *entry; + + entry =get_table_entry (table_info[category].table, id); +if (!entry) +return unknown_msg (category); +#if defined(USE_HOSTCC ) || !defined(CONFIG_NEEDS_MANUAL_RELOC ) +return entry->lname; +#else +return entry->lname + gd->reloc_off; +#endif +} + +/** +* get_cat_table_entry_short_name - translate entry id to short name +* @category: category to look up (enum ih_category) +* @id: entry id to be translated +* +* This will scan the translation table trying to find the entry that matches +* the given id. +* +* @retur short entry name if translation succeeds; error string on failure +*/ +const char *genimg_get_cat_short_name (enum ih_category category,uint id) +{ +const table_entry_t *entry; + + entry =get_table_entry (table_info[category].table, id); +if (!entry) +return unknown_msg (category); +#if defined(USE_HOSTCC ) || !defined(CONFIG_NEEDS_MANUAL_RELOC ) +return entry->sname; +#else +return entry->sname + gd->reloc_off; +#endif +} + +int genimg_get_cat_count (enum ih_category category) +{ +return table_info[category].count; +} + +const char *genimg_get_cat_desc (enum ih_category category) +{ +return table_info[category].desc; +} +/** * get_table_entry_name - translate entry id to long name * @table: pointer to a translation table for entries of a specific type
include/image.h +34 -0
@@ -478,6 +478,40 @@ const char *genimg_get_comp_name(uint8_t comp);*/ const char *genimg_get_comp_short_name (uint8_t comp); +/** +* genimg_get_cat_name() - Get the name of an item in a category +* +* @category: Category of item +* @id: Item ID +* @return name of item, or "Unknown ..." if unknown +*/ +const char *genimg_get_cat_name (enum ih_category category,uint id); + +/** +* genimg_get_cat_short_name() - Get the short name of an item in a category +* +* @category: Category of item +* @id: Item ID +* @return short name of item, or "Unknown ..." if unknown +*/ +const char *genimg_get_cat_short_name (enum ih_category category,uint id); + +/** +* genimg_get_cat_count() - Get the number of items in a category +* +* @category: Category to check +* @return the number of items in the category (IH_xxx_COUNT) +*/ +int genimg_get_cat_count (enum ih_category category); + +/** +* genimg_get_cat_desc() - Get the description of a category +* +* @return the description of a category, e.g. "architecture". This +* effectively converts the enum to a string. +*/ +const char *genimg_get_cat_desc (enum ih_category category); +int genimg_get_os_id (const char *name);int genimg_get_arch_id (const char *name);int genimg_get_type_id (const char *name);
Signature · 1 line
2.8.0.rc3.226.g39d4020
PATCH 10/14 image: Add functions to obtain category information
Quoted 8 lines · Simon Glass
Add generic functions which can look up information about a category:
- the number of items in the category
- the category description
- an item long time
- an item short timeSigned-off-by Simon Glass <[email protected]>
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/c782e7d0/attachment.sig>
PATCH 10/14 image: Add functions to obtain category information
In reply to Simon GlassQuoted 9 lines · Simon Glass
Add generic functions which can look up information about a category:
- the number of items in the category
- the category description
- an item long time
- an item short timeSigned-off-by Simon Glass <[email protected]>Reviewed-by Tom Rini <[email protected]>
Applied to u-boot/master, thanks!
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160716/298e5c0d/attachment.sig>
PATCH 11/14 mkimage: Allow display of a list of any image header category
In reply to Simon GlassAdd a generic function which can display a list of items in any category. This will allow displaying of images for the -A, -C, -O and -T flags. At present only -T is supported.
---
tools/mkimage.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+)
tools/mkimage.c +43 -0
@@ -25,6 +25,49 @@ static struct image_tool_params params = { .imagename2 = "", }; +static enum ih_category cur_category; + +static int h_compare_category_name (const void *vtype1,const void *vtype2) +{ +const int *type1 = vtype1; +const int *type2 = vtype2; +const char *name1 =genimg_get_cat_short_name (cur_category, *type1); +const char *name2 =genimg_get_cat_short_name (cur_category, *type2); + +return strcmp (name1, name2); +} + +int show_valid_options (enum ih_category category) +{ +int *order; +int count; +int item; +int i; + + count =genimg_get_cat_count (category); + order =calloc (count,sizeof (*order)); +if (!order) +return -ENOMEM ; + +/* Sort the names in order of short name for easier reading */ +for (item =0 ; item < count; item++) + order[item] = item; + cur_category = category; +qsort (order, count,sizeof (int ), h_compare_category_name); + +fprintf (stderr,"\nInvalid %s, supported are:\n" , +genimg_get_cat_desc (category)); +for (i =0 ; i < count; i++) { + item = order[i]; +fprintf (stderr,"\t%-15s %s\n" , +genimg_get_cat_short_name (category, item), +genimg_get_cat_name (category, item)); + } +fprintf (stderr,"\n" ); + +return 0 ; +} +static int h_compare_image_name (const void *vtype1,const void *vtype2) {const int *type1 = vtype1;
Signature · 1 line
2.8.0.rc3.226.g39d4020
PATCH 11/14 mkimage: Allow display of a list of any image header category
Quoted 5 lines · Simon Glass
Add a generic function which can display a list of items in any category. This will allow displaying of images for the -A, -C, -O and -T flags. At present only -T is supported.
Signed-off-by Simon Glass <[email protected]>
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/34836ce2/attachment.sig>
PATCH 11/14 mkimage: Allow display of a list of any image header category
In reply to Simon GlassQuoted 6 lines · Simon Glass
Add a generic function which can display a list of items in any category. This will allow displaying of images for the -A, -C, -O and -T flags. At present only -T is supported.
Signed-off-by Simon Glass <[email protected]>Reviewed-by Tom Rini <[email protected]>
Applied to u-boot/master, thanks!
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160716/bc331981/attachment.sig>
PATCH 12/14 mkimage: Use generic code for showing an 'image type' error
In reply to Simon GlassThe existing error code only displays image types which are claimed by a particular U_BOOT_IMAGE_TYPE() driver. But this does not seem correct. The mkimage tool should support all image types, so it makes sense to allow creation of images of any type with the tool.
When an incorrect image type is provided, use generic code to display the error.
---
tools/mkimage.c | 45 ++------------------------------------------- 1 file changed, 2 insertions(+), 43 deletions(-)
tools/mkimage.c +2 -43
@@ -37,7 +37,7 @@ static int h_compare_category_name(const void *vtype1, const void *vtype2)return strcmp (name1, name2); } -int show_valid_options (enum ih_category category) +static int show_valid_options (enum ih_category category) {int *order;int count;
@@ -68,47 +68,6 @@ int show_valid_options(enum ih_category category)return 0 ; } -static int h_compare_image_name (const void *vtype1,const void *vtype2) -{ -const int *type1 = vtype1; -const int *type2 = vtype2; -const char *name1 =genimg_get_type_short_name (*type1); -const char *name2 =genimg_get_type_short_name (*type2); - -return strcmp (name1, name2); -} - -/* Show all image types supported by mkimage */ -static void show_image_types (void ) -{ -struct image_type_params *tparams; -int order[IH_TYPE_COUNT ]; -int count; -int type; -int i; - -/* Sort the names in order of short name for easier reading */ -memset (order,'\0' ,sizeof (order)); -for (count =0 , type =0 ; type <IH_TYPE_COUNT ; type++) { - tparams =imagetool_get_type (type); -if (tparams) - order[count++] = type; - } -qsort (order, count,sizeof (int ), h_compare_image_name); - -fprintf (stderr,"\nInvalid image type. Supported image types:\n" ); -for (i =0 ; i < count; i++) { - type = order[i]; - tparams =imagetool_get_type (type); -if (tparams) { -fprintf (stderr,"\t%-15s %s\n" , -genimg_get_type_short_name (type), -genimg_get_type_name (type)); - } - } -fprintf (stderr,"\n" ); -} -static void usage (const char *msg) {fprintf (stderr,"Error: %s\n" , msg);
@@ -286,7 +245,7 @@ static void process_args(int argc, char **argv)case 'T' : type =genimg_get_type_id (optarg);if (type <0 ) { -show_image_types (); +show_valid_options (IH_TYPE );usage ("Invalid image type" ); }break ;
Signature · 1 line
2.8.0.rc3.226.g39d4020
PATCH 12/14 mkimage: Use generic code for showing an 'image type' error
Quoted 9 lines · Simon Glass
The existing error code only displays image types which are claimed by a particular
U_BOOT_IMAGE_TYPE()driver. But this does not seem correct. The mkimage tool should support all image types, so it makes sense to allow creation of images of any type with the tool.When an incorrect image type is provided, use generic code to display the error.
Signed-off-by Simon Glass <[email protected]>
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/4497133f/attachment.sig>
PATCH 12/14 mkimage: Use generic code for showing an 'image type' error
In reply to Simon GlassQuoted 10 lines · Simon Glass
The existing error code only displays image types which are claimed by a particular
U_BOOT_IMAGE_TYPE()driver. But this does not seem correct. The mkimage tool should support all image types, so it makes sense to allow creation of images of any type with the tool.When an incorrect image type is provided, use generic code to display the error.
Signed-off-by Simon Glass <[email protected]>Reviewed-by Tom Rini <[email protected]>
Applied to u-boot/master, thanks!
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160716/6acb9370/attachment.sig>
PATCH 13/14 mkimage: Show item lists for all categories
In reply to Simon GlassUpdate the error-handling code for -A, -C and -O to show a list of valid options when an invalid one is provided.
---
tools/mkimage.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
tools/mkimage.c +9 -3
@@ -152,8 +152,10 @@ static void process_args(int argc, char **argv)break ;case 'A' : params.arch =genimg_get_arch_id (optarg); -if (params.arch <0 ) +if (params.arch <0 ) { +show_valid_options (IH_ARCH );usage ("Invalid architecture" ); + }break ;case 'b' :if (add_content (IH_TYPE_FLATDT , optarg)) {
@@ -168,8 +170,10 @@ static void process_args(int argc, char **argv)break ;case 'C' : params.comp =genimg_get_comp_id (optarg); -if (params.comp <0 ) +if (params.comp <0 ) { +show_valid_options (IH_COMP );usage ("Invalid compression type" ); + }break ;case 'd' : params.datafile = optarg;
@@ -216,8 +220,10 @@ static void process_args(int argc, char **argv)break ;case 'O' : params.os =genimg_get_os_id (optarg); -if (params.os <0 ) +if (params.os <0 ) { +show_valid_options (IH_OS );usage ("Invalid operating system" ); + }break ;case 'p' : params.external_offset =strtoull (optarg, &ptr,16 );
Signature · 1 line
2.8.0.rc3.226.g39d4020
PATCH 13/14 mkimage: Show item lists for all categories
Quoted 5 lines · Simon Glass
Update the error-handling code for -A, -C and -O to show a list of valid options when an invalid one is provided.
Signed-off-by Simon Glass <[email protected]>Reported-by Vinoth Eswaran <[email protected]>
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/cb3f6d36/attachment.sig>
PATCH 13/14 mkimage: Show item lists for all categories
In reply to Simon GlassQuoted 6 lines · Simon Glass
Update the error-handling code for -A, -C and -O to show a list of valid options when an invalid one is provided.
Signed-off-by Simon Glass <[email protected]>Reported-by Vinoth Eswaran <[email protected]>Reviewed-by Tom Rini <[email protected]>
Applied to u-boot/master, thanks!
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160716/4aeacb34/attachment.sig>
PATCH 14/14 tools: Allow building with debug enabled
In reply to Simon GlassSometimes it is useful to build tools with debugging information included so that line-number information is available when run under gdb. Add a Kconfig option to support this.
---
Kconfig | 9 +++++++++ Makefile | 3 ++- 2 files changed, 11 insertions(+), 1 deletion(-)
Kconfig +9 -0
@@ -114,6 +114,15 @@ if EXPERT Warning: When disabling this, please check if malloc calls, maybe should be replaced by calloc - if one expects zeroed memory. + +config TOOLS_DEBUG + bool "Enable debug information for tools" + help + Enable generation of debug information for tools such as mkimage. + This can be used for debugging purposes. With debug information + it is possible to set breakpoints on particular lines, single-step + debug through the source code, etc. + endif endmenu # General setup
Makefile +2 -1
@@ -256,7 +256,8 @@ CONFIG_SHELL := $(shell if [ -x "$$BASH" ]; then echo $$BASH; \ HOSTCC = cc HOSTCXX = c++ -HOSTCFLAGS = -Wall -Wstrict-prototypes -O2 -fomit-frame-pointer +HOSTCFLAGS = -Wall -Wstrict-prototypes -O2 -fomit-frame-pointer \ + $(if $(CONFIG_TOOLS_DEBUG),-g) HOSTCXXFLAGS = -O2 ifeq ($(HOSTOS),cygwin)
Signature · 1 line
2.8.0.rc3.226.g39d4020
PATCH 14/14 tools: Allow building with debug enabled
Quoted 5 lines · Simon Glass
Sometimes it is useful to build tools with debugging information included so that line-number information is available when run under gdb. Add a
Kconfigoption to support this.Signed-off-by Simon Glass <[email protected]>
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/76a67164/attachment.sig>
PATCH 14/14 tools: Allow building with debug enabled
In reply to Simon GlassQuoted 6 lines · Simon Glass
Sometimes it is useful to build tools with debugging information included so that line-number information is available when run under gdb. Add a
Kconfigoption to support this.Signed-off-by Simon Glass <[email protected]>Reviewed-by Tom Rini <[email protected]>
Applied to u-boot/master, thanks!
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160716/7a11a4f1/attachment.sig>
cover mkimage: Tidy up error handling
In reply to Simon GlassHi Simon,
Quoted 5 lines · Simon Glass
There are a few problems when mkimage is provided with invalid arguments. In one case it crashes. When an invalid image type it is provided it lists the valid types, but this is not implemented for compression, architecture or OS.
There is another issue related to mkimage. It looks like it is broken since a lot of time, but it appears it was not noted.
mkimage -l is broken. Testing with i.MX images (imximage), it does not show the header, but it reports the output as it as a "gpimage".
In fact:
./tools/mkimage -l test.imx
GP Header: Size d1002040 LoadAddr 8017
It should be:
./tools/mkimage -l test.imx
Image Type: Freescale IMX Boot Image Image Ver: 2 (i.MX53/6/7 compatible) Data Size: 331776 Bytes = 324.00 kB = 0.32 MB Load Address: 177ff420 Entry Point: 17800000
The reason is due to the format of the gpimage itself. It has no magic number, and checking for its correctness means in gph_verify_header() just check that size and address are not NULL. That let think that the image is a gpimage, it is not. gpimage simply uses the image and does not let to check for further image headers that are put int the (sorted) list with the U_BOOT_IMAGE_TYPE. Image types that are in the list *before* gpimage are correctly recognized and printed, the following (as imximage) not.
A quick fix (maybe just for the release ?) should be to let gpimage (but I do not know if there is another image type that misbehaves as it does) as the last one in the list: if all detections fail, the last one without any possibility for detection (gpimage) runs. The following patch works for me:
tools/Makefile +2 -2
@@ -76,8 +76,6 @@ dumpimage-mkimage-objs := aisimage.o \ lib/fdtdec.o \ fit_common.o \ fit_image.o \ - gpimage.o \ - gpimage-common.o \ common/image-fit.o \ image-host.o \ common/image.o \
@@ -100,6 +98,8 @@ dumpimage-mkimage-objs := aisimage.o \ zynqimage.o \ zynqmpimage.o \ $(LIBFDT_OBJS) \ + gpimage.o \ + gpimage-common.o \ $(RSA_OBJS-y) dumpimage-objs := $(dumpimage-mkimage-objs) dumpimage.o
What do you think ? If this could be a solution for release, I send a formal patch.
Best regards,
Stefano
Quoted 27 lines
This series tidies this up a little, to make mkimage more friendly.
Simon Glass (14):
mkimage: Honour the default image type with auto-fit
mkimage: Explain the auto-fit imagefile special case
mkimage: Require a data file when auto-fit is used
mkimage: Drop premature setting of params.fit_image_type
mkimage: Drop blank line beforemain()
image: Correct auto-fit architecture property name
image: Convert theIH_... values to enums
image: Create a table of information for each category
image: Add a name for invalid types
image: Add functions to obtain category information
mkimage: Allow display of a list of any image header category
mkimage: Use generic code for showing an 'image type' error
mkimage: Show item lists for all categories
tools: Allow building with debug enabledKconfig | 9 +++ Makefile | 3 +- common/image.c | 87 ++++++++++++++++++++- include/image.h | 230 ++++++++++++++++++++++++++++++++++-------------------- tools/fit_image.c | 3 +- tools/mkimage.c | 69 +++++++++------- 6 files changed, 280 insertions(+), 121 deletions(-)
Signature · 5 lines
===================================================================== DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de =====================================================================
cover mkimage: Tidy up error handling
Quoted 67 lines · Stefano Babic
Hi Simon,
Quoted 5 lines · Simon Glass
There are a few problems when mkimage is provided with invalid arguments. In one case it crashes. When an invalid image type it is provided it lists the valid types, but this is not implemented for compression, architecture or OS.
There is another issue related to mkimage. It looks like it is broken since a lot of time, but it appears it was not noted.
mkimage -l is broken. Testing with i.MX images (imximage), it does not show the header, but it reports the output as it as a "gpimage".
In fact:
./tools/mkimage-l test.imx
GP Header: Sized1002040LoadAddr 8017It should be:
./tools/mkimage-l test.imxImage Type: Freescale IMX Boot Image Image Ver: 2 (i.MX53/6/7 compatible) Data Size: 331776 Bytes = 324.00 kB = 0.32 MB Load Address: 177ff420 Entry Point: 17800000The reason is due to the format of the gpimage itself. It has no magic number, and checking for its correctness means in
gph_verify_header()just check that size and address are not NULL. That let think that the image is a gpimage, it is not. gpimage simply uses the image and does not let to check for further image headers that are put int the (sorted) list with theU_BOOT_IMAGE_TYPE. Image types that are in the list *before* gpimage are correctly recognized and printed, the following (as imximage) not.A quick fix (maybe just for the release ?) should be to let gpimage (but I do not know if there is another image type that misbehaves as it does) as the last one in the list: if all detections fail, the last one without any possibility for detection (gpimage) runs. The following patch works for me:
tools/Makefile +2
-2@@ -76,8 +76,6 @@ dumpimage-mkimage-objs := aisimage.o \ lib/fdtdec.o \ fit_common.o \ fit_image.o \ - gpimage.o \ - gpimage-common.o \ common/image-fit.o \ image-host.o \ common/image.o \@@ -100,6 +98,8 @@ dumpimage-mkimage-objs := aisimage.o \ zynqimage.o \ zynqmpimage.o \ $(LIBFDT_OBJS) \ + gpimage.o \ + gpimage-common.o \ $(RSA_OBJS-y) dumpimage-objs := $(dumpimage-mkimage-objs) dumpimage.o
Quoted 2 lines
What do you think ? If this could be a solution for release, I send a formal patch.
Ug, I think we need what you're saying at least for release. I'm kicking off a big bisect now to see just when this last worked right.
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/69a97215/attachment.sig>
cover mkimage: Tidy up error handling
Hi,
Quoted 66 lines · Tom Rini
Quoted 65 lines · Stefano Babic
Hi Simon,
Quoted 5 lines · Simon Glass
There are a few problems when mkimage is provided with invalid arguments. In one case it crashes. When an invalid image type it is provided it lists the valid types, but this is not implemented for compression, architecture or OS.
There is another issue related to mkimage. It looks like it is broken since a lot of time, but it appears it was not noted.
mkimage -l is broken. Testing with i.MX images (imximage), it does not show the header, but it reports the output as it as a "gpimage".
In fact:
./tools/mkimage-l test.imx
GP Header: Sized1002040LoadAddr 8017It should be:
./tools/mkimage-l test.imxImage Type: Freescale IMX Boot Image Image Ver: 2 (i.MX53/6/7 compatible) Data Size: 331776 Bytes = 324.00 kB = 0.32 MB Load Address: 177ff420 Entry Point: 17800000The reason is due to the format of the gpimage itself. It has no magic number, and checking for its correctness means in
gph_verify_header()just check that size and address are not NULL. That let think that the image is a gpimage, it is not. gpimage simply uses the image and does not let to check for further image headers that are put int the (sorted) list with theU_BOOT_IMAGE_TYPE. Image types that are in the list *before* gpimage are correctly recognized and printed, the following (as imximage) not.A quick fix (maybe just for the release ?) should be to let gpimage (but I do not know if there is another image type that misbehaves as it does) as the last one in the list: if all detections fail, the last one without any possibility for detection (gpimage) runs. The following patch works for me:
tools/Makefile +2
-2@@ -76,8 +76,6 @@ dumpimage-mkimage-objs := aisimage.o \ lib/fdtdec.o \ fit_common.o \ fit_image.o \ - gpimage.o \ - gpimage-common.o \ common/image-fit.o \ image-host.o \ common/image.o \@@ -100,6 +98,8 @@ dumpimage-mkimage-objs := aisimage.o \ zynqimage.o \ zynqmpimage.o \ $(LIBFDT_OBJS) \ + gpimage.o \ + gpimage-common.o \ $(RSA_OBJS-y)
Quoted 7 lines
Quoted 4 lines
tools/Makefile +0
-0dumpimage-objs := $(dumpimage-mkimage-objs) dumpimage.oWhat do you think ? If this could be a solution for release, I send a formal patch.
Ug, I think we need what you're saying at least for release. I'm kicking off a big bisect now to see just when this last worked right.
That patch seems reasonable to me for now. My series is intended for the next release.
Perhaps verify_header() should return a value meaning 'possibly'?
Regards,
Simon
cover mkimage: Tidy up error handling
Quoted 77 lines · Simon Glass
Hi,
Quoted 66 lines · Tom Rini
Quoted 65 lines · Stefano Babic
Hi Simon,
Quoted 5 lines · Simon Glass
There are a few problems when mkimage is provided with invalid arguments. In one case it crashes. When an invalid image type it is provided it lists the valid types, but this is not implemented for compression, architecture or OS.
There is another issue related to mkimage. It looks like it is broken since a lot of time, but it appears it was not noted.
mkimage -l is broken. Testing with i.MX images (imximage), it does not show the header, but it reports the output as it as a "gpimage".
In fact:
./tools/mkimage-l test.imx
GP Header: Sized1002040LoadAddr 8017It should be:
./tools/mkimage-l test.imxImage Type: Freescale IMX Boot Image Image Ver: 2 (i.MX53/6/7 compatible) Data Size: 331776 Bytes = 324.00 kB = 0.32 MB Load Address: 177ff420 Entry Point: 17800000The reason is due to the format of the gpimage itself. It has no magic number, and checking for its correctness means in
gph_verify_header()just check that size and address are not NULL. That let think that the image is a gpimage, it is not. gpimage simply uses the image and does not let to check for further image headers that are put int the (sorted) list with theU_BOOT_IMAGE_TYPE. Image types that are in the list *before* gpimage are correctly recognized and printed, the following (as imximage) not.A quick fix (maybe just for the release ?) should be to let gpimage (but I do not know if there is another image type that misbehaves as it does) as the last one in the list: if all detections fail, the last one without any possibility for detection (gpimage) runs. The following patch works for me:
tools/Makefile +2
-2@@ -76,8 +76,6 @@ dumpimage-mkimage-objs := aisimage.o \ lib/fdtdec.o \ fit_common.o \ fit_image.o \ - gpimage.o \ - gpimage-common.o \ common/image-fit.o \ image-host.o \ common/image.o \@@ -100,6 +98,8 @@ dumpimage-mkimage-objs := aisimage.o \ zynqimage.o \ zynqmpimage.o \ $(LIBFDT_OBJS) \ + gpimage.o \ + gpimage-common.o \ $(RSA_OBJS-y)Quoted 7 lines
Quoted 4 lines
tools/Makefile +0
-0dumpimage-objs := $(dumpimage-mkimage-objs) dumpimage.oWhat do you think ? If this could be a solution for release, I send a formal patch.
Ug, I think we need what you're saying at least for release. I'm kicking off a big bisect now to see just when this last worked right.
Quoted 4 lines
That patch seems reasonable to me for now. My series is intended for the next release.
Perhaps
verify_header()should return a value meaning 'possibly'?
So, this was broken by:
commit 0ca6691c2ec20ff264d882854c339795579991f5
Author: Guilherme Maciel Ferreira <[email protected]> Date: Thu Jan 15 02:48:05 2015 -0200
imagetool: move common code to imagetool module
And yes, perhaps some way to say to say "maybe" is enough. Or maybe we just need to allow some to say that they can't verify? I think we're thinking along the same lines.
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/f5c39d49/attachment.sig>
cover mkimage: Tidy up error handling
Hi Tom, Simon,
Quoted 81 lines · Tom Rini
Quoted 80 lines · Simon Glass
Hi,
Quoted 66 lines · Tom Rini
Quoted 65 lines · Stefano Babic
Hi Simon,
Quoted 5 lines · Simon Glass
There are a few problems when mkimage is provided with invalid arguments. In one case it crashes. When an invalid image type it is provided it lists the valid types, but this is not implemented for compression, architecture or OS.
There is another issue related to mkimage. It looks like it is broken since a lot of time, but it appears it was not noted.
mkimage -l is broken. Testing with i.MX images (imximage), it does not show the header, but it reports the output as it as a "gpimage".
In fact:
./tools/mkimage-l test.imx
GP Header: Sized1002040LoadAddr 8017It should be:
./tools/mkimage-l test.imxImage Type: Freescale IMX Boot Image Image Ver: 2 (i.MX53/6/7 compatible) Data Size: 331776 Bytes = 324.00 kB = 0.32 MB Load Address: 177ff420 Entry Point: 17800000The reason is due to the format of the gpimage itself. It has no magic number, and checking for its correctness means in
gph_verify_header()just check that size and address are not NULL. That let think that the image is a gpimage, it is not. gpimage simply uses the image and does not let to check for further image headers that are put int the (sorted) list with theU_BOOT_IMAGE_TYPE. Image types that are in the list *before* gpimage are correctly recognized and printed, the following (as imximage) not.A quick fix (maybe just for the release ?) should be to let gpimage (but I do not know if there is another image type that misbehaves as it does) as the last one in the list: if all detections fail, the last one without any possibility for detection (gpimage) runs. The following patch works for me:
tools/Makefile +2
-2@@ -76,8 +76,6 @@ dumpimage-mkimage-objs := aisimage.o \ lib/fdtdec.o \ fit_common.o \ fit_image.o \ - gpimage.o \ - gpimage-common.o \ common/image-fit.o \ image-host.o \ common/image.o \@@ -100,6 +98,8 @@ dumpimage-mkimage-objs := aisimage.o \ zynqimage.o \ zynqmpimage.o \ $(LIBFDT_OBJS) \ + gpimage.o \ + gpimage-common.o \ $(RSA_OBJS-y)Quoted 7 lines
Quoted 4 lines
tools/Makefile +0
-0dumpimage-objs := $(dumpimage-mkimage-objs) dumpimage.oWhat do you think ? If this could be a solution for release, I send a formal patch.
Ug, I think we need what you're saying at least for release. I'm kicking off a big bisect now to see just when this last worked right.
That patch seems reasonable to me for now. My series is intended for the next release.
Quoted 9 lines
Quoted 1 line
Perhaps
verify_header()should return a value meaning 'possibly'?So, this was broken by:
commit0ca6691c2ec20ff264d882854c339795579991f5Author: Guilherme Maciel Ferreira <[email protected]> Date: Thu Jan 15 02:48:05 2015 -0200imagetool: move common code to imagetool module
Yes, I had bisected myself and I get this commit - but the patch is not the readon of the breakage.
Quoted 1 line
And yes, perhaps some way to say to say "maybe" is enough.
Right, but there is a side effect. Even if a "maybe" is returned (or let's say, a "I can't check"), the effect is that "GP Header: Size XXXX" is still printed, because for the gpimage this *could* be a correct image.
Quoted 3 lines
Or maybe we
just need to allow some to say that they can't verify? I think we're thinking along the same lines.
Regards,
Stefano
Signature · 5 lines
===================================================================== DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de =====================================================================
cover mkimage: Tidy up error handling
Hi Stefano,
Quoted 94 lines · Stefano Babic
Hi Tom, Simon,
Quoted 81 lines · Tom Rini
Quoted 80 lines · Simon Glass
Hi,
Quoted 66 lines · Tom Rini
Quoted 65 lines · Stefano Babic
Hi Simon,
Quoted 5 lines · Simon Glass
There are a few problems when mkimage is provided with invalid arguments. In one case it crashes. When an invalid image type it is provided it lists the valid types, but this is not implemented for compression, architecture or OS.
There is another issue related to mkimage. It looks like it is broken since a lot of time, but it appears it was not noted.
mkimage -l is broken. Testing with i.MX images (imximage), it does not show the header, but it reports the output as it as a "gpimage".
In fact:
./tools/mkimage-l test.imx
GP Header: Sized1002040LoadAddr 8017It should be:
./tools/mkimage-l test.imxImage Type: Freescale IMX Boot Image Image Ver: 2 (i.MX53/6/7 compatible) Data Size: 331776 Bytes = 324.00 kB = 0.32 MB Load Address: 177ff420 Entry Point: 17800000The reason is due to the format of the gpimage itself. It has no magic number, and checking for its correctness means in
gph_verify_header()just check that size and address are not NULL. That let think that the image is a gpimage, it is not. gpimage simply uses the image and does not let to check for further image headers that are put int the (sorted) list with theU_BOOT_IMAGE_TYPE. Image types that are in the list *before* gpimage are correctly recognized and printed, the following (as imximage) not.A quick fix (maybe just for the release ?) should be to let gpimage (but I do not know if there is another image type that misbehaves as it does) as the last one in the list: if all detections fail, the last one without any possibility for detection (gpimage) runs. The following patch works for me:
tools/Makefile +2
-2@@ -76,8 +76,6 @@ dumpimage-mkimage-objs := aisimage.o \ lib/fdtdec.o \ fit_common.o \ fit_image.o \ - gpimage.o \ - gpimage-common.o \ common/image-fit.o \ image-host.o \ common/image.o \@@ -100,6 +98,8 @@ dumpimage-mkimage-objs := aisimage.o \ zynqimage.o \ zynqmpimage.o \ $(LIBFDT_OBJS) \ + gpimage.o \ + gpimage-common.o \ $(RSA_OBJS-y)Quoted 7 lines
Quoted 4 lines
tools/Makefile +0
-0dumpimage-objs := $(dumpimage-mkimage-objs) dumpimage.oWhat do you think ? If this could be a solution for release, I send a formal patch.
Ug, I think we need what you're saying at least for release. I'm kicking off a big bisect now to see just when this last worked right.
That patch seems reasonable to me for now. My series is intended for the next release.
Quoted 9 lines
Quoted 1 line
Perhaps
verify_header()should return a value meaning 'possibly'?So, this was broken by:
commit0ca6691c2ec20ff264d882854c339795579991f5Author: Guilherme Maciel Ferreira <[email protected]> Date: Thu Jan 15 02:48:05 2015 -0200imagetool: move common code to imagetool module
Quoted 8 lines
Yes, I had bisected myself and I get this commit - but the patch is not the readon of the breakage.
Quoted 1 line
And yes, perhaps some way to say to say "maybe" is enough.
Right, but there is a side effect. Even if a "maybe" is returned (or let's say, a "I can't check"), the effect is that "GP Header: Size XXXX" is still printed, because for the gpimage this *could* be a correct image.
Well, verifying whether this is 'your' image should not print anything. We probably need to clarify the API a little. The comment for verify_header() could say that printing is not allowed.
Quoted 4 lines
Quoted 4 lines
Or maybe we
just need to allow some to say that they can't verify? I think we're thinking along the same lines.
Regards,
Simon