[PATCH u-boot-marvell v3 24/39] tools: kwbimage: Refactor kwbimage header size determination
Stefan Roese
sr at denx.de
Fri Oct 1 08:23:45 CEST 2021
On 24.09.21 23:07, Marek Behún wrote:
> From: Marek Behún <marek.behun at nic.cz>
>
> Add functions kwbheader_size() and kwbheader_size_for_csum().
>
> Refactor code determining header size to use these functions.
>
> Refactor header checksum determining function.
>
> Remove stuff that is not needed anymore.
>
> This simplifies the code a little and fixes one instance of validating
> header size meant for checksum instead of whole header size.
>
> Signed-off-by: Marek Behún <marek.behun at nic.cz>
Reviewed-by: Stefan Roese <sr at denx.de>
Thanks,
Stefan
> ---
> tools/kwbimage.c | 29 +++++++----------------------
> tools/kwbimage.h | 35 +++++++++++++++++++++++------------
> tools/kwboot.c | 22 ++++++++++------------
> 3 files changed, 40 insertions(+), 46 deletions(-)
>
> diff --git a/tools/kwbimage.c b/tools/kwbimage.c
> index 944a108cee..d1f4f93e0f 100644
> --- a/tools/kwbimage.c
> +++ b/tools/kwbimage.c
> @@ -280,14 +280,6 @@ static uint8_t image_checksum8(void *start, uint32_t len)
> return csum;
> }
>
> -size_t kwbimage_header_size(unsigned char *ptr)
> -{
> - if (kwbimage_version((void *)ptr) == 0)
> - return sizeof(struct main_hdr_v0);
> - else
> - return KWBHEADER_V1_SIZE((struct main_hdr_v1 *)ptr);
> -}
> -
> /*
> * Verify checksum over a complete header that includes the checksum field.
> * Return 1 when OK, otherwise 0.
> @@ -298,7 +290,7 @@ static int main_hdr_checksum_ok(void *hdr)
> struct main_hdr_v0 *main_hdr = (struct main_hdr_v0 *)hdr;
> uint8_t checksum;
>
> - checksum = image_checksum8(hdr, kwbimage_header_size(hdr));
> + checksum = image_checksum8(hdr, kwbheader_size_for_csum(hdr));
> /* Calculated checksum includes the header checksum field. Compensate
> * for that.
> */
> @@ -1649,8 +1641,8 @@ static int kwbimage_check_image_types(uint8_t type)
> static int kwbimage_verify_header(unsigned char *ptr, int image_size,
> struct image_tool_params *params)
> {
> - uint8_t checksum;
> - size_t header_size = kwbimage_header_size(ptr);
> + size_t header_size = kwbheader_size(ptr);
> + uint8_t csum;
>
> if (header_size > image_size)
> return -FDT_ERR_BADSTRUCTURE;
> @@ -1663,17 +1655,10 @@ static int kwbimage_verify_header(unsigned char *ptr, int image_size,
> struct main_hdr_v0 *mhdr = (struct main_hdr_v0 *)ptr;
>
> if (mhdr->ext & 0x1) {
> - struct ext_hdr_v0 *ext_hdr;
> -
> - if (header_size + sizeof(*ext_hdr) > image_size)
> - return -FDT_ERR_BADSTRUCTURE;
> + struct ext_hdr_v0 *ext_hdr = (void *)(mhdr + 1);
>
> - ext_hdr = (struct ext_hdr_v0 *)
> - (ptr + sizeof(struct main_hdr_v0));
> - checksum = image_checksum8(ext_hdr,
> - sizeof(struct ext_hdr_v0)
> - - sizeof(uint8_t));
> - if (checksum != ext_hdr->checksum)
> + csum = image_checksum8(ext_hdr, sizeof(*ext_hdr) - 1);
> + if (csum != ext_hdr->checksum)
> return -FDT_ERR_BADSTRUCTURE;
> }
> } else if (kwbimage_version(ptr) == 1) {
> @@ -1832,7 +1817,7 @@ static int kwbimage_generate(struct image_tool_params *params,
> static int kwbimage_extract_subimage(void *ptr, struct image_tool_params *params)
> {
> struct main_hdr_v1 *mhdr = (struct main_hdr_v1 *)ptr;
> - size_t header_size = kwbimage_header_size(ptr);
> + size_t header_size = kwbheader_size(ptr);
> struct opt_hdr_v1 *ohdr;
> int idx = params->pflag;
> int cur_idx = 0;
> diff --git a/tools/kwbimage.h b/tools/kwbimage.h
> index 738034beb1..56a748d4cf 100644
> --- a/tools/kwbimage.h
> +++ b/tools/kwbimage.h
> @@ -69,11 +69,6 @@ struct ext_hdr_v0 {
> uint8_t checksum;
> } __packed;
>
> -struct kwb_header {
> - struct main_hdr_v0 kwb_hdr;
> - struct ext_hdr_v0 kwb_exthdr;
> -} __packed;
> -
> /* Structure of the main header, version 1 (Armada 370/38x/XP) */
> struct main_hdr_v1 {
> uint8_t blockid; /* 0x0 */
> @@ -195,13 +190,6 @@ struct register_set_hdr_v1 {
> #define OPT_HDR_V1_BINARY_TYPE 0x2
> #define OPT_HDR_V1_REGISTER_TYPE 0x3
>
> -#define KWBHEADER_V0_SIZE(hdr) \
> - (((hdr)->ext & 0x1) ? sizeof(struct kwb_header) : \
> - sizeof(struct main_hdr_v0))
> -
> -#define KWBHEADER_V1_SIZE(hdr) \
> - (((hdr)->headersz_msb << 16) | le16_to_cpu((hdr)->headersz_lsb))
> -
> enum kwbimage_cmd {
> CMD_INVALID,
> CMD_BOOT_FROM,
> @@ -235,6 +223,29 @@ static inline unsigned int kwbimage_version(const void *header)
> return ptr[8];
> }
>
> +static inline size_t kwbheader_size(const void *header)
> +{
> + if (kwbimage_version(header) == 0) {
> + const struct main_hdr_v0 *hdr = header;
> +
> + return sizeof(*hdr) +
> + (hdr->ext & 0x1) ? sizeof(struct ext_hdr_v0) : 0;
> + } else {
> + const struct main_hdr_v1 *hdr = header;
> +
> + return (hdr->headersz_msb << 16) |
> + le16_to_cpu(hdr->headersz_lsb);
> + }
> +}
> +
> +static inline size_t kwbheader_size_for_csum(const void *header)
> +{
> + if (kwbimage_version(header) == 0)
> + return sizeof(struct main_hdr_v0);
> + else
> + return kwbheader_size(header);
> +}
> +
> static inline uint32_t opt_hdr_v1_size(const struct opt_hdr_v1 *ohdr)
> {
> return (ohdr->headersz_msb << 16) | le16_to_cpu(ohdr->headersz_lsb);
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index e47bf94e89..e60f7c5b7a 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -583,10 +583,7 @@ kwboot_xmodem(int tty, const void *_img, size_t size)
> int rc, pnum;
> size_t hdrsz;
>
> - if (kwbimage_version(img) == 0)
> - hdrsz = KWBHEADER_V0_SIZE((struct main_hdr_v0 *)img);
> - else
> - hdrsz = KWBHEADER_V1_SIZE((struct main_hdr_v1 *)img);
> + hdrsz = kwbheader_size(img);
>
> kwboot_printv("Waiting 2s and flushing tty\n");
> sleep(2); /* flush isn't effective without it */
> @@ -746,9 +743,13 @@ out:
> }
>
> static uint8_t
> -kwboot_img_csum8(void *_data, size_t size)
> +kwboot_hdr_csum8(const void *hdr)
> {
> - uint8_t *data = _data, csum;
> + const uint8_t *data = hdr;
> + uint8_t csum;
> + size_t size;
> +
> + size = kwbheader_size_for_csum(hdr);
>
> for (csum = 0; size-- > 0; data++)
> csum += *data;
> @@ -794,17 +795,14 @@ kwboot_img_patch_hdr(void *img, size_t size)
> goto out;
> }
>
> - if (image_ver == 0)
> - hdrsz = sizeof(*hdr);
> - else
> - hdrsz = KWBHEADER_V1_SIZE(hdr);
> + hdrsz = kwbheader_size(hdr);
>
> if (size < hdrsz) {
> errno = EINVAL;
> goto out;
> }
>
> - csum = kwboot_img_csum8(hdr, hdrsz) - hdr->checksum;
> + csum = kwboot_hdr_csum8(hdr) - hdr->checksum;
> if (csum != hdr->checksum) {
> errno = EINVAL;
> goto out;
> @@ -860,7 +858,7 @@ kwboot_img_patch_hdr(void *img, size_t size)
> hdr->blockid = IBR_HDR_UART_ID;
> }
>
> - hdr->checksum = kwboot_img_csum8(hdr, hdrsz) - csum;
> + hdr->checksum = kwboot_hdr_csum8(hdr) - csum;
>
> rc = 0;
> out:
>
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de
More information about the U-Boot
mailing list