On 12.01.24 16:26, Stefan Kerkmann wrote:
> Hello Ahmad,
>
> On 11.01.24 16:57, Ahmad Fatoum wrote:
>> Hello Stefan,
>>
>>> diff --git a/drivers/hab/habv4.c b/drivers/hab/habv4.c
>>> index ed6d4db77c..dfa0207435 100644
>>> --- a/drivers/hab/habv4.c
>>> +++ b/drivers/hab/habv4.c
>>> @@ -144,31 +144,41 @@ struct hab_header {
>>> uint8_t par;
>>> } __packed;
>>> -typedef enum hab_status hab_loader_callback_fn(void **start, uint32_t *bytes, const void *boot_data);
>>> +typedef enum hab_status hab_loader_callback_fn(void **start, size_t *bytes, const void *boot_data);
>>> +typedef void (*hab_image_entry_fn)(void);
>>
>> While it only matter for forward declaration, perhaps, change this
>> to be a non-pointer like hab_loader_callback_fn above?
>
> Done in v2.
>
>>> + enum hab_status (*run_dcd)(const uint8_t *dcd);
>>> + enum hab_status (*run_csf)(const uint8_t *csf, uint8_t cid, uint32_t srkmask);
>>> enum hab_status (*assert)(enum hab_assertion assertion, const void *data, uint32_t count);
>>> - enum hab_status (*report_event)(enum hab_status status, uint32_t index, void *event, uint32_t *bytes);
>>> + enum hab_status (*report_event)(enum hab_status status, uint32_t index, uint8_t *event, size_t *bytes);
>>> enum hab_status (*report_status)(enum hab_config *config, enum habv4_state *state);
>>> void (*failsafe)(void);
>>> + hab_image_entry_fn(* authenticate_image_no_dcd)(uint8_t cid, ptrdiff_t ivt_offset, void **start, size_t *bytes, hab_loader_callback_fn loader);
>>
>> And here you rely on a function pointer being automatically derived.
>> While that's ok from a correctness point of view, for symmetry, it
>> would be better to stick to one type.
>
> Done in v2.
>
>>> + uint32_t(* get_version)(void);
>>
>> Nitpick: space after uint32_t and not before get_version.
>>
>
> Done in v2.
>
>>> + enum hab_status (*authenticate_container)(uint8_t cid, ptrdiff_t ivt_offset, void **start, size_t *bytes, hab_loader_callback_fn loader, uint32_t srkmask, int skip_dcd);
>>> } __packed;
>>> -#define FSL_SIP_HAB 0xC2000007
>>
>> This is removed without replacement?
>>
>
> No it is still there, the diff is a bit misleading.
Ah, its whitespace was changed..
>
> Regards,
> Stefan
>
>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
