Am 09.09.2015 um 06:33 schrieb Siarhei Siamashka:
+uint8_t get_image_type(const uint8_t *buf, int error)
IMHO, this is not the nicest looking API. We get a buffer pointer here,
but not the buffer size, so it is implicitly assumed to be large enough.
The use of the error flag is not very readable in the code, that is
calling this function if it is just 0 or 1.
[...]
If the 'get_image_type()' function is going to be called from multiple
places, then this error message text is too generic and does not provide
enough context. Maybe error reporting should be just done by the caller
of this function?
Or maybe this whole function can be replaced with just a structure
declaration (if the primary intention is to avoid using the magic array
indexes too much)?
So far, we're only making use of the ih_arch and ih_type fields (plus
the ih_magic). Currently I'd prefer to avoid duplicating the header
structure declaration (from U-Boot's image.h).
This function declaration is a bit awkward, yes. I agree that the
error handling inside the function might be lacking 'context', and as
right now there's only one place where it would matter (the checks
within aw_fel_write_uboot_image), we might as well adapt your proposal.
I'd like to change the function to
int get_image_type(const uint8_t *buf, size_t len);
As ih_type is limited in range, we could make use of (our own) negative
constants to indicate various error conditions. A signature mismatch
(or insufficient length) would still return IH_TYPE_INVALID, and we
might indicate some IH_TYPE_ARCH_MISMATCH by -1.
The caller can either examine specific return values, or just use
(get_image_type() > IH_TYPE_INVALID) to test for valid images.
Regards, B. Nortmann
--
You received this message because you are subscribed to the Google Groups
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
