[U-Boot] [PATCH v2 3/4] tools: mkimage: kwbimage list command support

[Wolfgang Denk]

Dear Prafulla Wadaskar,

In message <1248220911-11378-1-git-send-email-prafulla at marvell.com> you wrote:
> Signed-off-by: Prafulla Wadaskar <prafulla at marvell.com>
...
> --- a/common/image.c
> +++ b/common/image.c
> @@ -159,7 +159,7 @@ static table_entry_t uimage_comp[] = {
>  
>  uint32_t crc32 (uint32_t, const unsigned char *, uint);
>  uint32_t crc32_wd (uint32_t, const unsigned char *, uint, uint);
> -static void genimg_print_size (uint32_t size);
> +void genimg_print_size (uint32_t size);

When exporting this function, then please add prototype to header
file.

>  #if defined(CONFIG_TIMESTAMP) || defined(CONFIG_CMD_DATE) || defined(USE_HOSTCC)
>  static void genimg_print_time (time_t timestamp);
>  #endif
> @@ -473,7 +473,7 @@ void memmove_wd (void *to, void *from, size_t len, ulong chunksz)
>  }
>  #endif /* !USE_HOSTCC */
>  
> -static void genimg_print_size (uint32_t size)
> +void genimg_print_size (uint32_t size)
>  {
>  #ifndef USE_HOSTCC
>  	printf ("%d Bytes = ", size);
> diff --git a/tools/kwbimage.c b/tools/kwbimage.c
> index f78d97a..9441a19 100644
> --- a/tools/kwbimage.c
> +++ b/tools/kwbimage.c
> @@ -33,6 +33,8 @@
>  #include <unistd.h>
>  #include "kwbimage.h"
>  
> +extern void genimg_print_size (uint32_t size);

Please delete and use prototype from header file.

>  static struct kwb_header kwbimage_header;
>  static int datacmd_cnt = 0;
>  
> @@ -262,3 +264,42 @@ void kwbimage_set_header (struct kwb_header *hdr, struct stat *sbuf,
>  	exthdr->checkSum = kwbimage_checksum8 ((void *)exthdr,
>  						sizeof(extbhr_t), 0);
>  }
> +
> +/* -l support functions */
> +int kwbimage_check_header (struct kwb_header *hdr)
> +{
> +	bhr_t *mhdr = &hdr->kwb_hdr;
> +	extbhr_t *exthdr = &hdr->kwb_exthdr;
> +	uint8_t calc_hdrcsum;
> +	uint8_t calc_exthdrcsum;
> +
> +	calc_hdrcsum = kwbimage_checksum8 ((void *)mhdr,
> +			sizeof(bhr_t) - sizeof(uint8_t), 0);
> +	if (calc_hdrcsum != mhdr->checkSum)
> +		return MHDR_CSUM_NOT_MATCHED;
> +
> +	calc_exthdrcsum = kwbimage_checksum8 ((void *)exthdr,
> +			sizeof(extbhr_t) - sizeof(uint8_t), 0);
> +	if (calc_hdrcsum != mhdr->checkSum)
> +		return MHDR_CSUM_NOT_MATCHED;
> +
> +	return HDR_CSUM_MATCHED;
> +}
> +
> +void kwbimage_print_contents (struct kwb_header *hdr) {
> +	bhr_t *mhdr = &hdr->kwb_hdr;
> +	char * bootdev = "Unknown";
> +
> +	if (mhdr->blockid == IBR_HDR_SPI_ID)
> +		bootdev = "spi";
> +	if (mhdr->blockid == IBR_HDR_NAND_ID)
> +		bootdev = "nand";
> +	if (mhdr->blockid == IBR_HDR_SATA_ID)
> +		bootdev = "sata";
> +
> +	printf ("Image Type:   Kirkwood Boot from %s Image\n", bootdev);
> +	printf ("Data Size:    ");
> +	genimg_print_size (mhdr->blocksize - sizeof(uint32_t));
> +	printf ("Load Address: %08x\n", mhdr->destaddr);
> +	printf ("Entry Point:  %08x\n", mhdr->execaddr);
> +}
> diff --git a/tools/kwbimage.h b/tools/kwbimage.h
> index 57db29f..d32f293 100644
> --- a/tools/kwbimage.h
> +++ b/tools/kwbimage.h
> @@ -55,6 +55,12 @@ enum kwbimage_cmd {
>  	CMD_DDR_DATA
>  };
>  
> +enum kwbimage_err {
> +	EHDR_CSUM_NOT_MATCHED = -2,
> +	MHDR_CSUM_NOT_MATCHED,
> +	HDR_CSUM_MATCHED
> +};
> +
>  /* typedefs */
>  typedef struct bhr_t {
>  	uint8_t blockid;		/*0     */
> diff --git a/tools/mkimage.c b/tools/mkimage.c
> index d77d1d6..71076fa 100644
> --- a/tools/mkimage.c
> +++ b/tools/mkimage.c
> @@ -166,7 +166,7 @@ NXTARG:		;
>  		usage();
>  
>  	if (((argc != 1) && (opt_type == IH_TYPE_KWBIMAGE)) &&
> -		(xflag || lflag))
> +		(xflag))
>  		usage();

Thanks. Please ignore my question in patch 2/4 :-)

>  	if (!eflag) {
> @@ -231,6 +231,11 @@ NXTARG:		;
>  			exit (EXIT_FAILURE);
>  		}
>  
> +		if (!(kwbimage_check_header ((struct kwb_header *)ptr))) {
> +			kwbimage_print_contents ((struct kwb_header *)ptr);
> +			exit (EXIT_SUCCESS);

I think you should add error checking here. For example, when the code
detects a corruption of the image (checksum error or the like), it
should return an error code here, so you can use "mkimage -l" to
actually verify the consistency of an image (in automated scripts).


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
Unix is like a toll road on which you have to stop every 50  feet  to
pay another nickel. But hey! You only feel 5 cents poorer each time.
                 - Larry Wall in <1992Aug13.192357.15731 at netlabs.com>