Re: [PATCH v2] kwboot: do a filetype check before sending the image

[Trent Piepho <tpiepho@kymetacorp.com>]

On Thu, 2016-01-21 at 11:13 +0100, Uwe Kleine-König wrote:

> > And having compared the version tag to 1, couldn't you then cast img
> > into a struct main_hdr_v1 *?  That would avoid all the hard coded magic
> > offsets in the rest of the code.  Unless img isn't aligned?
> 
> ... or the build machine is big endian. (Note in this case kwbimage is
> broken, too. And given that catching such an error independently is IMHO
> a feature.) I'd prefer to keep it as is at least for now for a few
> reasons (endianess, alignment, bigger reshuffling of code because said
> struct is defined in a different .c file)

Endianess shouldn't make a difference when it comes to using the struct.
Seems like it would be nice to have a common definition of the structs
between the code that makes the image and the code the reads it.


> >> +
> >> +	image_size = img[0x4] | (img[0x5] << 8) |
> >> +		(img[0x6] << 16) | (img[0x7] << 24);
> > 
> > struct main_hdr_v1 *hdr = (const struct main_hdr
> > image_size = le32_to_cpu(hdr->blocksize);
> > or if unaligned:
> > image_size = get_unaligned_le32(&img[0x4]);
> 
> Of the three offered versions I'd prefer mine ... It's simple and easy
> to verify when comparing to the reference manual.
> 
> >> +
> >> +	header_size = (img[0x9] << 16) | img[0xa] | (img[0xb] << 8);
> > 
> > header_size = hdr->headersz_msb << 16 | le16_to_cpu(hdr->headersz_lsb);
> > 
> >> +
> >> +	if (header_size + image_size != size) {
> >> +		printf("Size mismatch (%zu + %zu != %zu)\n",
> >> +		       header_size, image_size, size);
> >> +		goto err;
> >> +	} else {
> > 
> > Don't really need the else block here since the failure above exits,
> > just like the two failure checks before this one.
> 
> right, this is a relict of v1 where I didn't jump in the first block.
> 
> >> +		for (i = 0; i < header_size; ++i)
> >> +			csum += img[i];
> > 
> > csum = image_checksum8(img, header_size)
> 
> This is again in kwbimage.c
> 
> Best regards
> Uwe
> 

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox